Lucene - Core
  1. Lucene - Core
  2. LUCENE-3522

TermsFilter.getDocIdSet(context) NPE on missing field

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 4.0-ALPHA
    • Component/s: modules/other
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      If the context does not contain the field for a term when calling TermsFilter.getDocIdSet(AtomicReaderContext context) then a NullPointerException is thrown due to not checking for null Terms before getting iterator.

        Activity

        Hide
        Shai Erera added a comment -

        Ah. I thought that we need the Fix Version to properly track which issues are part of a release. But you're right - if this bug didn't exist in 3.x, then we better not mark that it was fixed there.

        Show
        Shai Erera added a comment - Ah. I thought that we need the Fix Version to properly track which issues are part of a release. But you're right - if this bug didn't exist in 3.x, then we better not mark that it was fixed there.
        Hide
        Michael McCandless added a comment -

        OK that's fine but technically this bug didn't exist on 3.5... I only backported the test case.

        Show
        Michael McCandless added a comment - OK that's fine but technically this bug didn't exist on 3.5... I only backported the test case.
        Hide
        Shai Erera added a comment -

        Added 3.5 as a fix version as well

        Show
        Shai Erera added a comment - Added 3.5 as a fix version as well
        Hide
        Michael McCandless added a comment -

        Dan/Shai feel free to fix the test case if you want... didn't see your comments here until after I committed!

        Show
        Michael McCandless added a comment - Dan/Shai feel free to fix the test case if you want... didn't see your comments here until after I committed!
        Hide
        Michael McCandless added a comment -

        Thanks Dan!

        I committed to trunk and backported the test case to 3.x. I had to add missing rd1/2.close() at the end of the test case.

        Show
        Michael McCandless added a comment - Thanks Dan! I committed to trunk and backported the test case to 3.x. I had to add missing rd1/2.close() at the end of the test case.
        Hide
        Shai Erera added a comment -

        Good catch Dan!

        Patch looks good, but I have some comments about the test:

        1. You don't close the directories at the end of it, and the test fails due that.
        2. I think it can be simplified to create just one Directory, with "f1:content" and request "f2:content". I actually tried it and the test still fails (NPE reproduced without your fix).

        Here is the modified, more compact, test:

          public void testMissingField() throws Exception {
            // LUCENE-3522: if requested field does not exist in the index, TermsFilter threw NPE.
            Directory dir = newDirectory();
            RandomIndexWriter writer = new RandomIndexWriter(random, dir);
            Document doc = new Document();
            doc.add(newField("f1", "content", StringField.TYPE_STORED));
            writer.addDocument(doc);
            IndexReader reader = writer.getReader();
            writer.close();
            
            TermsFilter tf = new TermsFilter();
            tf.addTerm(new Term("f2", "content"));
            
            FixedBitSet bits = (FixedBitSet) tf.getDocIdSet(reader.getTopReaderContext().leaves()[0]);
            assertTrue("Must be >= 0", bits.cardinality() >= 0);      
            reader.close();
            dir.close();
          }
        

        Would you mind changing the test case to this compact one? Or did you want to demonstrate something else with the two readers?

        Show
        Shai Erera added a comment - Good catch Dan! Patch looks good, but I have some comments about the test: You don't close the directories at the end of it, and the test fails due that. I think it can be simplified to create just one Directory, with "f1:content" and request "f2:content". I actually tried it and the test still fails (NPE reproduced without your fix). Here is the modified, more compact, test: public void testMissingField() throws Exception { // LUCENE-3522: if requested field does not exist in the index, TermsFilter threw NPE. Directory dir = newDirectory(); RandomIndexWriter writer = new RandomIndexWriter(random, dir); Document doc = new Document(); doc.add(newField( "f1" , "content" , StringField.TYPE_STORED)); writer.addDocument(doc); IndexReader reader = writer.getReader(); writer.close(); TermsFilter tf = new TermsFilter(); tf.addTerm( new Term( "f2" , "content" )); FixedBitSet bits = (FixedBitSet) tf.getDocIdSet(reader.getTopReaderContext().leaves()[0]); assertTrue( "Must be >= 0" , bits.cardinality() >= 0); reader.close(); dir.close(); } Would you mind changing the test case to this compact one? Or did you want to demonstrate something else with the two readers?
        Hide
        Dan Climan added a comment -

        Proposed patch to TermsFilter and TermsFilterTest showing problem and demonstrating fix

        Show
        Dan Climan added a comment - Proposed patch to TermsFilter and TermsFilterTest showing problem and demonstrating fix

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Dan Climan
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development