Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9838

atomic "inc" when adding doc doesn't respect field "default" from schema

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.5, 7.0
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      If you do an "atomic update" when adding a document for the first time, then the "inc" operator acts as if the field has a default of 0.

      But if the <field/> has an actual default in the schema.xml (example: default="42") then that default is ignored by the atomic update code path.

      1. SOLR-9838.patch
        3 kB
        Ishan Chattopadhyaya
      2. SOLR-9838.patch
        4 kB
        Amrit Sarkar

        Issue Links

          Activity

          Hide
          hossman Hoss Man added a comment -

          I noticed this while working on SOLR-5944, as part of that issue i'll add an @AwaitsFix test that points to this jira, but the general gist of the problem is that if you do something like this...

          clearIndex();
          assertU(adoc(sdoc("id", "7", "field_with_default", ImmutableMap.of("inc", "666"))));
          

          ..the end result is that doc#7 gets a value of 666 in "field_with_default" instead of 666 + whatever the default field value is.

          Show
          hossman Hoss Man added a comment - I noticed this while working on SOLR-5944 , as part of that issue i'll add an @AwaitsFix test that points to this jira, but the general gist of the problem is that if you do something like this... clearIndex(); assertU(adoc(sdoc("id", "7", "field_with_default", ImmutableMap.of("inc", "666")))); ..the end result is that doc#7 gets a value of 666 in "field_with_default" instead of 666 + whatever the default field value is.
          Hide
          sarkaramrit2@gmail.com Amrit Sarkar added a comment -

          The troubled code is in AtomicUpdateDocumentMerger.java::doInc(..) :

            protected void doInc(SolrInputDocument toDoc, SolrInputField sif, Object fieldVal) {
              SolrInputField numericField = toDoc.get(sif.getName());
              if (numericField == null) {
                toDoc.setField(sif.getName(),  fieldVal); //need to check the default in schema here, instead just putting whatever is coming
              } else {
               ...................................................................
            }
          

          I will try to cook up a patch for the same with relevant test cases.

          Show
          sarkaramrit2@gmail.com Amrit Sarkar added a comment - The troubled code is in AtomicUpdateDocumentMerger.java::doInc(..) : protected void doInc(SolrInputDocument toDoc, SolrInputField sif, Object fieldVal) { SolrInputField numericField = toDoc.get(sif.getName()); if (numericField == null) { toDoc.setField(sif.getName(), fieldVal); //need to check the default in schema here, instead just putting whatever is coming } else { ................................................................... } I will try to cook up a patch for the same with relevant test cases.
          Hide
          sarkaramrit2@gmail.com Amrit Sarkar added a comment -

          Hoss,

          SOLR-9838.patch uploaded which incorporates default values while performing "inc" operation. Fixed a single test-method too.

          	modified:   solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java
          	modified:   solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java

          Feedback will be appreciated.

          Show
          sarkaramrit2@gmail.com Amrit Sarkar added a comment - Hoss, SOLR-9838 .patch uploaded which incorporates default values while performing "inc" operation. Fixed a single test-method too. modified: solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java modified: solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java Feedback will be appreciated.
          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment -

          The change looks good. Attaching another patch with some refactoring / cleanup. Planning to commit this soon.

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - The change looks good. Attaching another patch with some refactoring / cleanup. Planning to commit this soon.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit a06c39f3e50a1616cd7c48f4454dd77be17c7278 in lucene-solr's branch refs/heads/master from Ishan Chattopadhyaya
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a06c39f ]

          SOLR-9838: 'inc' atomic update doesn't respect default field value

          Show
          jira-bot ASF subversion and git services added a comment - Commit a06c39f3e50a1616cd7c48f4454dd77be17c7278 in lucene-solr's branch refs/heads/master from Ishan Chattopadhyaya [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a06c39f ] SOLR-9838 : 'inc' atomic update doesn't respect default field value
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit dab1e2d66c9232a938ab8d6fdd1032f1984c75be in lucene-solr's branch refs/heads/branch_6x from Ishan Chattopadhyaya
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=dab1e2d ]

          SOLR-9838: 'inc' atomic update doesn't respect default field value

          Show
          jira-bot ASF subversion and git services added a comment - Commit dab1e2d66c9232a938ab8d6fdd1032f1984c75be in lucene-solr's branch refs/heads/branch_6x from Ishan Chattopadhyaya [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=dab1e2d ] SOLR-9838 : 'inc' atomic update doesn't respect default field value
          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment -

          Thanks Amrit Sarkar.

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - Thanks Amrit Sarkar .

            People

            • Assignee:
              ichattopadhyaya Ishan Chattopadhyaya
              Reporter:
              hossman Hoss Man
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development