Solr
  1. Solr
  2. SOLR-6510

select?collapse=... - fix NPE in Collapsing(FieldValue|Score)Collector

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.8.1
    • Fix Version/s: 4.10.3
    • Component/s: None
    • Labels:
      None

      Description

      Affects branch_4x but not trunk, collapse field must be docValues=true and shard empty (or with nothing indexed for the field?).

        Issue Links

          Activity

          Hide
          ASF GitHub Bot added a comment -

          GitHub user cpoerschke opened a pull request:

          https://github.com/apache/lucene-solr/pull/94

          select?collapse=... - fix NPE in Collapsing(FieldValue|Score)Collector

          https://issues.apache.org/jira/i#browse/SOLR-6510

          Patch is against branch_4x where CollapsingQParserPlugin line 230's searcher.getAtomicReader().getSortedDocValues call could return null. On trunk the line 230 call has been replaced (by LUCENE-5666) and so the CollapsingQParserPlugin part of the patch is not required.

          The TestCollapseQParserPlugin change could go to branch_4x only or branch_4x and trunk.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/bloomberg/lucene-solr branch_4x-collapse-empty

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/94.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #94


          commit 5e2968541221307a6ca7b397c61a040289421ac1
          Author: Christine Poerschke <cpoerschke@bloomberg.net>
          Date: 2014-09-05T17:00:38Z

          solr: select?collapse=... - fix NPE in Collapsing(FieldValue|Score)Collector


          Show
          ASF GitHub Bot added a comment - GitHub user cpoerschke opened a pull request: https://github.com/apache/lucene-solr/pull/94 select?collapse=... - fix NPE in Collapsing(FieldValue|Score)Collector https://issues.apache.org/jira/i#browse/SOLR-6510 Patch is against branch_4x where CollapsingQParserPlugin line 230's searcher.getAtomicReader().getSortedDocValues call could return null. On trunk the line 230 call has been replaced (by LUCENE-5666 ) and so the CollapsingQParserPlugin part of the patch is not required. The TestCollapseQParserPlugin change could go to branch_4x only or branch_4x and trunk. You can merge this pull request into a Git repository by running: $ git pull https://github.com/bloomberg/lucene-solr branch_4x-collapse-empty Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/94.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #94 commit 5e2968541221307a6ca7b397c61a040289421ac1 Author: Christine Poerschke <cpoerschke@bloomberg.net> Date: 2014-09-05T17:00:38Z solr: select?collapse=... - fix NPE in Collapsing(FieldValue|Score)Collector
          Hide
          Joel Bernstein added a comment -

          Thanks for the patch Christine.

          It looks I can call the FieldCache.getTermsIndex() and it should protect against the null. I'll test that out. If that works, I'll just remove the direct call to AtomicReader.getSortedDocValues(), which was probably never needed in the first place.

          Show
          Joel Bernstein added a comment - Thanks for the patch Christine. It looks I can call the FieldCache.getTermsIndex() and it should protect against the null. I'll test that out. If that works, I'll just remove the direct call to AtomicReader.getSortedDocValues(), which was probably never needed in the first place.
          Hide
          David Smiley added a comment -

          I'd like to see this get fixed for the next Solr release. Joel; if you're not going to be able to do it, I'll step in. Just let me know.

          Show
          David Smiley added a comment - I'd like to see this get fixed for the next Solr release. Joel; if you're not going to be able to do it, I'll step in. Just let me know.
          Hide
          Joel Bernstein added a comment -

          If I've been following things correctly the next release will be cut from branch_5x. The CollapsingQParserPlugin has a different implementation for 5.0 that uses the DocValues class, which I believe does not have this issue.

          More testing needs to be done with 5.0 implementation to understand the full effect of this change. The change was made as part of a large patch when the new DocValues class was introduced to Lucene.

          I'll open another ticket as a review for the CollapsingQParserPlugin 5.0.

          We can probably close this ticket though.

          Show
          Joel Bernstein added a comment - If I've been following things correctly the next release will be cut from branch_5x. The CollapsingQParserPlugin has a different implementation for 5.0 that uses the DocValues class, which I believe does not have this issue. More testing needs to be done with 5.0 implementation to understand the full effect of this change. The change was made as part of a large patch when the new DocValues class was introduced to Lucene. I'll open another ticket as a review for the CollapsingQParserPlugin 5.0. We can probably close this ticket though.
          Hide
          Joel Bernstein added a comment -

          If there is going to be a 4.10.2, then we could fix this bug for that release though.

          Show
          Joel Bernstein added a comment - If there is going to be a 4.10.2, then we could fix this bug for that release though.
          Hide
          Ramkumar Aiyengar added a comment -

          I see bugs getting tagged for a 4.10.2, though that may not be indicative of a release..

          Show
          Ramkumar Aiyengar added a comment - I see bugs getting tagged for a 4.10.2, though that may not be indicative of a release..
          Hide
          Joel Bernstein added a comment -

          Ok, let's assume they'll be a 4.10.2.

          David, if you can take this one that will be great. Trunk and 5x shouldn't be affected by this though and I'll confirm this while working on SOLR-6581.

          Show
          Joel Bernstein added a comment - Ok, let's assume they'll be a 4.10.2. David, if you can take this one that will be great. Trunk and 5x shouldn't be affected by this though and I'll confirm this while working on SOLR-6581 .
          Hide
          David Smiley added a comment -

          I looked into this a bit, applying Christine's patch. It appears that there may be a bug in MultiDocValues (a DocValues view over multiple atomic segments). If there is no value, you don't get -1 for the ord, and hence the test complains. The BytesRef shows to be the empty string. I'll dig into this more later.

          Show
          David Smiley added a comment - I looked into this a bit, applying Christine's patch. It appears that there may be a bug in MultiDocValues (a DocValues view over multiple atomic segments). If there is no value, you don't get -1 for the ord, and hence the test complains. The BytesRef shows to be the empty string. I'll dig into this more later.
          Hide
          David Smiley added a comment -

          It'll take awhile for me to return to this, but the reproducing test failure for me is: -Dtests.seed=1EFA19CE63B33EA:4C1BC8B5A1E50CDA The first query in testCollapseQueries failed. I ran the test a bunch of times previously without failure, so this trip isn't common.

          Show
          David Smiley added a comment - It'll take awhile for me to return to this, but the reproducing test failure for me is: -Dtests.seed=1EFA19CE63B33EA:4C1BC8B5A1E50CDA The first query in testCollapseQueries failed. I ran the test a bunch of times previously without failure, so this trip isn't common.
          Hide
          David Smiley added a comment -

          Aha. The problem is that all the codecs are in rotation, to include Lucene42 which didn't support DocValues with empty values. I'll add a suitable @SuppressCodecs list to the test and get this committed tonight.

          Show
          David Smiley added a comment - Aha. The problem is that all the codecs are in rotation, to include Lucene42 which didn't support DocValues with empty values. I'll add a suitable @SuppressCodecs list to the test and get this committed tonight.
          Hide
          ASF subversion and git services added a comment -

          Commit 1642620 from David Smiley in branch 'dev/trunk'
          [ https://svn.apache.org/r1642620 ]

          SOLR-6510: add tests for collapse on docvalues

          Show
          ASF subversion and git services added a comment - Commit 1642620 from David Smiley in branch 'dev/trunk' [ https://svn.apache.org/r1642620 ] SOLR-6510 : add tests for collapse on docvalues
          Hide
          ASF subversion and git services added a comment -

          Commit 1642621 from David Smiley in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1642621 ]

          SOLR-6510: add tests for collapse on docvalues

          Show
          ASF subversion and git services added a comment - Commit 1642621 from David Smiley in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1642621 ] SOLR-6510 : add tests for collapse on docvalues
          Hide
          ASF subversion and git services added a comment -

          Commit 1642624 from David Smiley in branch 'dev/branches/lucene_solr_4_10'
          [ https://svn.apache.org/r1642624 ]

          SOLR-6510: fix collapse on docvalues, empty index

          Show
          ASF subversion and git services added a comment - Commit 1642624 from David Smiley in branch 'dev/branches/lucene_solr_4_10' [ https://svn.apache.org/r1642624 ] SOLR-6510 : fix collapse on docvalues, empty index
          Hide
          ASF GitHub Bot added a comment -

          Github user cpoerschke closed the pull request at:

          https://github.com/apache/lucene-solr/pull/94

          Show
          ASF GitHub Bot added a comment - Github user cpoerschke closed the pull request at: https://github.com/apache/lucene-solr/pull/94

            People

            • Assignee:
              David Smiley
              Reporter:
              Christine Poerschke
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development