Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.8
    • Fix Version/s: 4.9, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      While testing out the QueryRescorer I was getting an NPE on the scorer when using a TermQuery as the rescore query. Looks like a TermQuery will return a null Scorer if the term is not present in the index segment.

      Caused by: java.lang.NullPointerException
      [junit4] > at org.apache.lucene.search.QueryRescorer.rescore(QueryRescorer.java:89)

      1. LUCENE-5682.patch
        4 kB
        Joel Bernstein
      2. LUCENE-5682.patch
        3 kB
        Joel Bernstein
      3. LUCENE-5682.patch
        3 kB
        Joel Bernstein

        Activity

        Hide
        Michael McCandless added a comment -

        Ahh, yes, we need just need a null check when we pull the scorer. Do you want to make a test case / fix patch?

        Show
        Michael McCandless added a comment - Ahh, yes, we need just need a null check when we pull the scorer. Do you want to make a test case / fix patch?
        Hide
        Joel Bernstein added a comment -

        Sure, I'll assign this myself.

        Show
        Joel Bernstein added a comment - Sure, I'll assign this myself.
        Hide
        Joel Bernstein added a comment -

        Quick patch. It's a little tough to read due to some code indenting but basically I just wrapped a null check around this block of code:

              if(scorer != null) {
                int targetDoc = docID - docBase;
                int actualDoc = scorer.docID();
                if (actualDoc < targetDoc) {
                  actualDoc = scorer.advance(targetDoc);
                }
        
                if (actualDoc == targetDoc) {
                  // Query did match this doc:
                  hit.score = combine(hit.score, true, scorer.score());
                } else {
                  // Query did not match this doc:
                  assert actualDoc > targetDoc;
                  hit.score = combine(hit.score, false, 0.0f);
                }
              }
        

        Looks like we need to check for a null scorer on each hit because the segment could change with each hit. I didn't follow through with a combine is this situation as you did on a miss hit, but we could if you want to.

        Show
        Joel Bernstein added a comment - Quick patch. It's a little tough to read due to some code indenting but basically I just wrapped a null check around this block of code: if (scorer != null ) { int targetDoc = docID - docBase; int actualDoc = scorer.docID(); if (actualDoc < targetDoc) { actualDoc = scorer.advance(targetDoc); } if (actualDoc == targetDoc) { // Query did match this doc: hit.score = combine(hit.score, true , scorer.score()); } else { // Query did not match this doc: assert actualDoc > targetDoc; hit.score = combine(hit.score, false , 0.0f); } } Looks like we need to check for a null scorer on each hit because the segment could change with each hit. I didn't follow through with a combine is this situation as you did on a miss hit, but we could if you want to.
        Hide
        Michael McCandless added a comment -

        Thanks Joel!

        I didn't follow through with a combine is this situation as you did on a miss hit, but we could if you want to.

        I think we should? So subclass doesn't see these two cases as any different? Ie, in both cases, the 2nd pass Query didn't match the doc.

        Show
        Michael McCandless added a comment - Thanks Joel! I didn't follow through with a combine is this situation as you did on a miss hit, but we could if you want to. I think we should? So subclass doesn't see these two cases as any different? Ie, in both cases, the 2nd pass Query didn't match the doc.
        Hide
        Joel Bernstein added a comment - - edited

        Ok, that makes sense attaching new patch with this logic:

         if(scorer != null) {
             int targetDoc = docID - docBase;
             int actualDoc = scorer.docID();
             if (actualDoc < targetDoc) {
                actualDoc = scorer.advance(targetDoc);
             }
        
              if (actualDoc == targetDoc) {
                 // Query did match this doc:
                  hit.score = combine(hit.score, true, scorer.score());
              } else {
                  // Query did not match this doc:
                  assert actualDoc > targetDoc;
                  hit.score = combine(hit.score, false, 0.0f);
              }
         } else {
            hit.score = combine(hit.score, false, 0.0f);
         }
        
        
        Show
        Joel Bernstein added a comment - - edited Ok, that makes sense attaching new patch with this logic: if (scorer != null ) { int targetDoc = docID - docBase; int actualDoc = scorer.docID(); if (actualDoc < targetDoc) { actualDoc = scorer.advance(targetDoc); } if (actualDoc == targetDoc) { // Query did match this doc: hit.score = combine(hit.score, true , scorer.score()); } else { // Query did not match this doc: assert actualDoc > targetDoc; hit.score = combine(hit.score, false , 0.0f); } } else { hit.score = combine(hit.score, false , 0.0f); }
        Hide
        Michael McCandless added a comment -

        +1, looks great.

        Maybe just add a comment "Query did not match this doc" in the new else clause? Thanks!

        Show
        Michael McCandless added a comment - +1, looks great. Maybe just add a comment "Query did not match this doc" in the new else clause? Thanks!
        Hide
        Joel Bernstein added a comment -

        OK, all set. Latest patch with comment and CHANGES.txt

        Looks like we missed 4.8.1. So I added to 4.9 in the CHANGES.txt.

        I'll commit shortly.

        Show
        Joel Bernstein added a comment - OK, all set. Latest patch with comment and CHANGES.txt Looks like we missed 4.8.1. So I added to 4.9 in the CHANGES.txt. I'll commit shortly.
        Hide
        ASF subversion and git services added a comment -

        Commit 1595973 from Joel Bernstein in branch 'dev/trunk'
        [ https://svn.apache.org/r1595973 ]

        LUCENE-5682: NPE in QueryRescorer when Scorer is null

        Show
        ASF subversion and git services added a comment - Commit 1595973 from Joel Bernstein in branch 'dev/trunk' [ https://svn.apache.org/r1595973 ] LUCENE-5682 : NPE in QueryRescorer when Scorer is null
        Hide
        ASF subversion and git services added a comment -

        Commit 1596009 from Joel Bernstein in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1596009 ]

        LUCENE-5682: NPE in QueryRescorer when Scorer is null

        Show
        ASF subversion and git services added a comment - Commit 1596009 from Joel Bernstein in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1596009 ] LUCENE-5682 : NPE in QueryRescorer when Scorer is null
        Hide
        Michael McCandless added a comment -

        I think this is now fixed?

        Show
        Michael McCandless added a comment - I think this is now fixed?
        Hide
        Joel Bernstein added a comment -

        Yes, thanks for resolving.

        Show
        Joel Bernstein added a comment - Yes, thanks for resolving.

          People

          • Assignee:
            Joel Bernstein
            Reporter:
            Joel Bernstein
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development