Lucene - Core
  1. Lucene - Core
  2. LUCENE-6950

DimensionalRangeQuery not working with UninvertingReader

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.5, 6.0, 5.4.1
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      As I was trying out dimensional fields for SOLR-8396, I realized that DimensionalRangeQuery is not working with UninvertingReader.
      In Solr, all directory readers are wrapped by an UninvertingReader and an ExitableDirectoryReader.

      Here's the error:

      Exception in thread "main" java.lang.IllegalArgumentException: field="rating" was indexed with numDims=0 but this query has numDims=1
      	at org.apache.lucene.search.DimensionalRangeQuery$1.scorer(DimensionalRangeQuery.java:186)
      	at org.apache.lucene.search.Weight.bulkScorer(Weight.java:135)
      	at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:667)
      	at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:474)
      	at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:593)
      	at org.apache.lucene.search.IndexSearcher.searchAfter(IndexSearcher.java:451)
      	at org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:462)
      	at DimensionalRangeQueryExample.query(DimensionalRangeQueryExample.java:66)
      

      Here's an example program to trigger this failure:

      import java.io.IOException;
      import java.util.HashMap;
      import java.util.Map;
      import java.util.Random;
      
      import org.apache.lucene.analysis.standard.StandardAnalyzer;
      import org.apache.lucene.document.DimensionalIntField;
      import org.apache.lucene.document.Document;
      import org.apache.lucene.document.Field;
      import org.apache.lucene.document.Field.Store;
      import org.apache.lucene.document.LegacyIntField;
      import org.apache.lucene.document.StringField;
      import org.apache.lucene.document.TextField;
      import org.apache.lucene.index.DirectoryReader;
      import org.apache.lucene.index.IndexWriter;
      import org.apache.lucene.index.IndexWriterConfig;
      import org.apache.lucene.index.StoredDocument;
      import org.apache.lucene.queryparser.classic.ParseException;
      import org.apache.lucene.search.DimensionalRangeQuery;
      import org.apache.lucene.search.IndexSearcher;
      import org.apache.lucene.search.LegacyNumericRangeQuery;
      import org.apache.lucene.search.Query;
      import org.apache.lucene.search.ScoreDoc;
      import org.apache.lucene.search.TopDocs;
      import org.apache.lucene.store.Directory;
      import org.apache.lucene.store.RAMDirectory;
      import org.apache.lucene.uninverting.UninvertingReader;
      import org.apache.lucene.util.BytesRef;
      
      public class DimensionalRangeQueryExample {
      	public static void main(String[] args) throws IOException, ParseException {
      		StandardAnalyzer analyzer = new StandardAnalyzer();
      		Directory index = new RAMDirectory();
      		IndexWriterConfig config = new IndexWriterConfig(analyzer);
      
      		IndexWriter w = new IndexWriter(index, config);
      		addDoc(w, "Lucene in Action", 1);
      		addDoc(w, "Lucene for Dummies", 2);
      		addDoc(w, "Managing Gigabytes", 3);
      		addDoc(w, "The Art of Computer Science", 4);
      		w.commit();
      		w.close();
      
      		DirectoryReader reader = (DirectoryReader.open(index));
      
      		Map<String, UninvertingReader.Type> uninvertingMap = new HashMap<>();
      		uninvertingMap.put("id", UninvertingReader.Type.BINARY);
      		uninvertingMap.put("rating", UninvertingReader.Type.INTEGER);
      		reader = UninvertingReader.wrap(reader, uninvertingMap);
      
      		IndexSearcher searcher = new IndexSearcher(reader);
      
      		Query legacyQuery = LegacyNumericRangeQuery.newIntRange("rating_legacy", 1, 4, true, true);
      		Query dimensionalQuery = DimensionalRangeQuery.new1DIntRange("rating", 1, true, 4, true);
      
      		System.out.println("Legacy query: ");
      		query(legacyQuery, searcher); // works
      		System.out.println("Dimensional query: ");
      		query(dimensionalQuery, searcher); // fails
      		
      		reader.close();
      	}
      
      	private static void query(Query q, IndexSearcher searcher) throws IOException {
      		int hitsPerPage = 10;
      		TopDocs docs = searcher.search(q, hitsPerPage);
      		ScoreDoc[] hits = docs.scoreDocs;
      
      		System.out.println("Found " + hits.length + " hits.");
      		for(int i=0;i<hits.length;++i) {
      			int docId = hits[i].doc;
      			StoredDocument d = searcher.doc(docId);
      			System.out.println((i + 1) + ". " + d);
      		}
      	}
      	private static void addDoc(IndexWriter w, String title, int rating) throws IOException {
      		Document doc = new Document();
      		String id = ""+new Random().nextInt(1000);
      		byte idBytes[] = id.getBytes();
      		doc.add(new StringField("id", new BytesRef(idBytes), Store.YES));
      		doc.add(new TextField("title", title, Field.Store.YES));
      		doc.add(new DimensionalIntField("rating", rating));
      		LegacyIntField legacy = new LegacyIntField("rating_legacy", rating, Store.YES);
      		legacy.setIntValue(rating);
      		doc.add(legacy);
      		w.addDocument(doc);
      	}
      }
      

      I don't yet know more as to why this could be. Michael McCandless Any ideas, please?
      Apologies if I should've brought this up in the mailing lists, or as a comment in LUCENE-6917 itself.

      1. LUCENE-6950.patch
        4 kB
        Ishan Chattopadhyaya
      2. LUCENE-6950.patch
        0.8 kB
        Ishan Chattopadhyaya
      3. LUCENE-6950.patch
        0.8 kB
        Ishan Chattopadhyaya
      4. LUCENE-6950.patch
        1 kB
        Ishan Chattopadhyaya

        Issue Links

          Activity

          Hide
          Ishan Chattopadhyaya added a comment -

          Here's a patch for the fix. No unit tests yet, but solves the problem for the example program I posted in the description.

          Show
          Ishan Chattopadhyaya added a comment - Here's a patch for the fix. No unit tests yet, but solves the problem for the example program I posted in the description.
          Hide
          Robert Muir added a comment -

          I don't think we need the conditional or the setter, the bug is that 0's are bogusly passed to both in the constructor?

          Show
          Robert Muir added a comment - I don't think we need the conditional or the setter, the bug is that 0's are bogusly passed to both in the constructor?
          Hide
          Ishan Chattopadhyaya added a comment -

          Thanks Rob! I was too sleepy to observe the constructor carefully Updated the patch.

          Show
          Ishan Chattopadhyaya added a comment - Thanks Rob! I was too sleepy to observe the constructor carefully Updated the patch.
          Hide
          Robert Muir added a comment -

          In addition to the dimensionCount=0 and dimensionNumBytes=0 causing your problem, the dvGen=-1 and attributes=Collections.emptyMap are also bogus. We should just pass everything, only difference is dvType comes from "type". Then there won't be strange bugs with it, and it won't hide information.

          Show
          Robert Muir added a comment - In addition to the dimensionCount=0 and dimensionNumBytes=0 causing your problem, the dvGen=-1 and attributes=Collections.emptyMap are also bogus. We should just pass everything, only difference is dvType comes from "type". Then there won't be strange bugs with it, and it won't hide information.
          Hide
          Robert Muir added a comment -

          I am just basing that on MismatchedLeafReader, which is very similar to this reader (only it changes a different variable, the field's number):

          http://svn.apache.org/repos/asf/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/MismatchedLeafReader.java

          It passes all the old stuff and only modifies its one thing.

          Show
          Robert Muir added a comment - I am just basing that on MismatchedLeafReader, which is very similar to this reader (only it changes a different variable, the field's number): http://svn.apache.org/repos/asf/lucene/dev/trunk/lucene/test-framework/src/java/org/apache/lucene/index/MismatchedLeafReader.java It passes all the old stuff and only modifies its one thing.
          Hide
          Ishan Chattopadhyaya added a comment -

          That makes sense, I updated the patch to pass those around too.

          I saw that at TermVectorLeafReader, there are 0s passed in for FieldInfo constructor, but I think it is as much information as we have there and it is okay. Just confirming my understanding.

          Also, Rob, can you please suggest what (or whether) tests should I write for this issue? I can try something on the lines of the example program I posted, but was wondering if something better can be done.

          Show
          Ishan Chattopadhyaya added a comment - That makes sense, I updated the patch to pass those around too. I saw that at TermVectorLeafReader, there are 0s passed in for FieldInfo constructor, but I think it is as much information as we have there and it is okay. Just confirming my understanding. Also, Rob, can you please suggest what (or whether) tests should I write for this issue? I can try something on the lines of the example program I posted, but was wondering if something better can be done.
          Hide
          Robert Muir added a comment -

          Thanks, the patch looks good.

          As far as testing this, We could add simple unit tests for what it is doing with fieldinfos in TestUninvertingReader. Similar to ones like "testSortedSetInteger", but even simpler. just tests that add field to a single document, wrap it, then assert stuff about the fieldinfos on the wrapped reader. Particularly asserts about docvalues type (which we change if an indexed field is configured for uninverting) would be good. We could have lots of tests for the logic in there, e.g. one that adds a stored-only field and then asserts dvType is still NONE, for example.

          Show
          Robert Muir added a comment - Thanks, the patch looks good. As far as testing this, We could add simple unit tests for what it is doing with fieldinfos in TestUninvertingReader. Similar to ones like "testSortedSetInteger", but even simpler. just tests that add field to a single document, wrap it, then assert stuff about the fieldinfos on the wrapped reader. Particularly asserts about docvalues type (which we change if an indexed field is configured for uninverting) would be good. We could have lots of tests for the logic in there, e.g. one that adds a stored-only field and then asserts dvType is still NONE, for example.
          Hide
          Ishan Chattopadhyaya added a comment -

          Updated the patch with the tests.

          Show
          Ishan Chattopadhyaya added a comment - Updated the patch with the tests.
          Hide
          Robert Muir added a comment -

          +1, thanks for adding those tests.

          Show
          Robert Muir added a comment - +1, thanks for adding those tests.
          Hide
          ASF subversion and git services added a comment -

          Commit 1722165 from Robert Muir in branch 'dev/trunk'
          [ https://svn.apache.org/r1722165 ]

          LUCENE-6950: Fix FieldInfos handling of UninvertingReader

          Show
          ASF subversion and git services added a comment - Commit 1722165 from Robert Muir in branch 'dev/trunk' [ https://svn.apache.org/r1722165 ] LUCENE-6950 : Fix FieldInfos handling of UninvertingReader
          Hide
          ASF subversion and git services added a comment -

          Commit 1722196 from Robert Muir in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1722196 ]

          LUCENE-6950: Fix FieldInfos handling of UninvertingReader

          Show
          ASF subversion and git services added a comment - Commit 1722196 from Robert Muir in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1722196 ] LUCENE-6950 : Fix FieldInfos handling of UninvertingReader
          Hide
          Robert Muir added a comment -

          Thanks Ishan!

          Show
          Robert Muir added a comment - Thanks Ishan!
          Hide
          Adrien Grand added a comment -

          Reopen for backporting to 5.4.1.

          Show
          Adrien Grand added a comment - Reopen for backporting to 5.4.1.
          Hide
          ASF subversion and git services added a comment -

          Commit 1724058 from Adrien Grand in branch 'dev/trunk'
          [ https://svn.apache.org/r1724058 ]

          LUCENE-6950: Move CHANGES entry to 5.4.1.

          Show
          ASF subversion and git services added a comment - Commit 1724058 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1724058 ] LUCENE-6950 : Move CHANGES entry to 5.4.1.
          Hide
          ASF subversion and git services added a comment -

          Commit 1724059 from Adrien Grand in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1724059 ]

          LUCENE-6950: Move CHANGES entry to 5.4.1.

          Show
          ASF subversion and git services added a comment - Commit 1724059 from Adrien Grand in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1724059 ] LUCENE-6950 : Move CHANGES entry to 5.4.1.
          Hide
          ASF subversion and git services added a comment -

          Commit 1724060 from Adrien Grand in branch 'dev/branches/lucene_solr_5_4'
          [ https://svn.apache.org/r1724060 ]

          LUCENE-6950: Fix FieldInfos handling of UninvertingReader

          Show
          ASF subversion and git services added a comment - Commit 1724060 from Adrien Grand in branch 'dev/branches/lucene_solr_5_4' [ https://svn.apache.org/r1724060 ] LUCENE-6950 : Fix FieldInfos handling of UninvertingReader

            People

            • Assignee:
              Unassigned
              Reporter:
              Ishan Chattopadhyaya
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development