Solr
  1. Solr
  2. SOLR-7679

Schema API doesn't take similarity attribute into account when adding field types

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.2
    • Fix Version/s: 5.3, 6.0
    • Component/s: Schema and Analysis
    • Labels:
      None

      Description

      When using the request

      curl -X POST -H 'Content-type:application/json' --data-binary '{
        "add-field-type": {
          "name": "fieldTypeWithSimilarity",
          "class": "org.apache.solr.schema.TextField",
          "analyzer": {
            "charFilters": [
              {
                "class": "solr.PatternReplaceCharFilterFactory",
                "replacement": "$1$1",
                "pattern": "([a-zA-Z])\\\\1+"
              }
            ],
            "tokenizer": {
              "class": "solr.WhitespaceTokenizerFactory"
            }
          },
          "similarity": {
            "class": "org.apache.lucene.misc.SweetSpotSimilarity"
          }
        }
      }' http://localhost:8983/solr/gettingstarted/schema
      
      

      can be seen in the updated schema.xml that the similarity attributes for the newly added field type doesn't contain a similarity entry.
      This is due to the fact that within FieldTypeXmlAdapter the similiarity element is not being added to the field type.

      1. SOLR-7679.patch
        5 kB
        Steve Rowe
      2. SOLR-7679.patch
        6 kB
        Marius Grama
      3. SOLR-7679.patch
        1 kB
        Marius Grama

        Issue Links

          Activity

          Hide
          Marius Grama added a comment -

          Here is a patch fixing this issue.
          Fix is done by creating the similarity element containing the sent attributes.
          No unit test provided as part of this ticket, but within SOLR-7182 (work in progress) there is a unit test for this specific situation.

          Show
          Marius Grama added a comment - Here is a patch fixing this issue. Fix is done by creating the similarity element containing the sent attributes. No unit test provided as part of this ticket, but within SOLR-7182 (work in progress) there is a unit test for this specific situation.
          Hide
          Steve Rowe added a comment -

          +1, patch looks good to me.

          Show
          Steve Rowe added a comment - +1, patch looks good to me.
          Hide
          Steve Rowe added a comment -

          No unit test provided as part of this ticket, but within SOLR-7182 (work in progress) there is a unit test for this specific situation.

          Marius, could you point out where the test is? It would be preferable to commit a test with this ticket (which will almost certainly go in first).

          Show
          Steve Rowe added a comment - No unit test provided as part of this ticket, but within SOLR-7182 (work in progress) there is a unit test for this specific situation. Marius, could you point out where the test is? It would be preferable to commit a test with this ticket (which will almost certainly go in first).
          Hide
          Marius Grama added a comment - - edited

          Steve Rowe I figured out that it is actualy easy to test the change I've made through TestBulkSchemaAPI and I've added in there a unit test.

          Attached a new version of the patch containing also a unit test for the changed functionality.

          Show
          Marius Grama added a comment - - edited Steve Rowe I figured out that it is actualy easy to test the change I've made through TestBulkSchemaAPI and I've added in there a unit test. Attached a new version of the patch containing also a unit test for the changed functionality.
          Hide
          Steve Rowe added a comment -

          Thanks Marius.

          I'm attaching a modified patch:

          You switched TestBulkSchemaAPI to using the to-be-deprecated (SOLR-6594) non-bulk API, via extra servlet setup in before(), AFAICT so that you could get the showDefaults behavior, which is not available with the bulk Schema API. But this is a bug in FieldType.getNamedPropertyValues(): it only outputs similarity when showDefaults=true, but it should instead always do so. My version of the patch fixes this bug. (This is a round-tripping problem: manual testing shows that when managed schema is persisted, per-field-type similarity is currently dropped, which is bad.)

          I was then able to revert TestBulkSchemaAPI to the previously used bulk schema API, since showDefaults is no longer necessary to get the per-field-type similarity to show up in the schema api response.

          I think this is ready. I'll commit after running Solr tests and precommit.

          In my testing I noticed that SchemaSimilarityFactory, a prerequisite for setting per-field-type similarity, is not the default in any of Solr's example schemas. See http://markmail.org/message/7icpmwmdhfw4tmwv and SOLR-3577 for rationale.

          We need to let people know that in order to set per-field-type similarity, they need to first set the global similarity to SchemaSimilarityFactory or something like it. Unfortunately the Schema API can't yet set the global similarity - see SOLR-7242. I'll leave a comment there about the need to check/update the Solr Reference Guide for this.

          Show
          Steve Rowe added a comment - Thanks Marius. I'm attaching a modified patch: You switched TestBulkSchemaAPI to using the to-be-deprecated ( SOLR-6594 ) non-bulk API, via extra servlet setup in before() , AFAICT so that you could get the showDefaults behavior, which is not available with the bulk Schema API. But this is a bug in FieldType.getNamedPropertyValues() : it only outputs similarity when showDefaults=true , but it should instead always do so. My version of the patch fixes this bug. (This is a round-tripping problem: manual testing shows that when managed schema is persisted, per-field-type similarity is currently dropped, which is bad.) I was then able to revert TestBulkSchemaAPI to the previously used bulk schema API, since showDefaults is no longer necessary to get the per-field-type similarity to show up in the schema api response. I think this is ready. I'll commit after running Solr tests and precommit. In my testing I noticed that SchemaSimilarityFactory , a prerequisite for setting per-field-type similarity, is not the default in any of Solr's example schemas. See http://markmail.org/message/7icpmwmdhfw4tmwv and SOLR-3577 for rationale. We need to let people know that in order to set per-field-type similarity, they need to first set the global similarity to SchemaSimilarityFactory or something like it. Unfortunately the Schema API can't yet set the global similarity - see SOLR-7242 . I'll leave a comment there about the need to check/update the Solr Reference Guide for this.
          Hide
          ASF subversion and git services added a comment -

          Commit 1686491 from Steve Rowe in branch 'dev/trunk'
          [ https://svn.apache.org/r1686491 ]

          SOLR-7679: Schema API doesn't take similarity attribute into account when adding field types

          Show
          ASF subversion and git services added a comment - Commit 1686491 from Steve Rowe in branch 'dev/trunk' [ https://svn.apache.org/r1686491 ] SOLR-7679 : Schema API doesn't take similarity attribute into account when adding field types
          Hide
          ASF subversion and git services added a comment -

          Commit 1686498 from Steve Rowe in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1686498 ]

          SOLR-7679: Schema API doesn't take similarity attribute into account when adding field types (merged trunk r1686491)

          Show
          ASF subversion and git services added a comment - Commit 1686498 from Steve Rowe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1686498 ] SOLR-7679 : Schema API doesn't take similarity attribute into account when adding field types (merged trunk r1686491)
          Hide
          Steve Rowe added a comment -

          Committed to trunk and branch_5x.

          Thanks Marius!

          Show
          Steve Rowe added a comment - Committed to trunk and branch_5x. Thanks Marius!
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Steve Rowe
              Reporter:
              Marius Grama
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development