You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "HoustonPutman (via GitHub)" <gi...@apache.org> on 2026/10/22 22:03:06 UTC

[PR] Skip index-sort range shortcuts on segments with blocks and multi-valued fields [lucene]

HoustonPutman opened a new pull request, #16893:
URL: https://github.com/apache/lucene/pull/16893

   Fixes https://github.com/apache/lucene/issues/16884, https://github.com/apache/lucene/issues/16885, https://github.com/apache/lucene/issues/16886.
   
   ### Description
   
   This fixes a few range query bugs regarding multi-valued fields and segments with blocks when index sorting is enabled on the range query field.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Skip index-sort range shortcuts on segments with blocks and multi-valued fields [lucene]

Posted by "romseygeek (via GitHub)" <gi...@apache.org>.
romseygeek commented on code in PR #16893:
URL: https://github.com/apache/lucene/pull/16893#discussion_r4375572205


##########
lucene/core/src/test/org/apache/lucene/search/TestIndexSortSortedNumericDocValuesRangeQuery.java:
##########
@@ -551,6 +555,144 @@ public void testMultiDocValues() throws Exception {
     dir.close();
   }
 
+  /**
+   * Weight#count must not use the index sort when a document has multiple values: the index sort
+   * only reflects one value per document (the min by default), but the range query matches a
+   * document if any of its values is in range.
+   */
+  public void testCountWithMultiValuedField() throws IOException {
+    for (boolean withMissingDoc : new boolean[] {false, true}) {
+      Directory dir = newDirectory();
+      IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random()));
+      iwc.setIndexSort(new Sort(new SortedNumericSortField("field", SortField.Type.LONG)));
+      RandomIndexWriter writer = new RandomIndexWriter(random(), dir, iwc);
+
+      if (withMissingDoc) {
+        // sorts as the missing value 0, which is outside the query range
+        writer.addDocument(createMissingValueDocument());
+      }
+      // these sort by their min value (1, 2 and 60) but match [50, 200] through their max value
+      long[][] docs = {{1, 100}, {2, 3}, {60, 70}};
+      for (long[] values : docs) {
+        Document doc = new Document();
+        for (long value : values) {
+          doc.add(new SortedNumericDocValuesField("field", value));
+        }
+        writer.addDocument(doc);
+      }
+      writer.forceMerge(1);
+
+      IndexReader reader = writer.getReader();
+      IndexSearcher searcher = newSearcher(reader, false);
+
+      Query query = createQuery("field", 50, 200);
+      assertEquals(
+          2, searcher.count(SortedNumericDocValuesField.newSlowRangeQuery("field", 50, 200)));
+      assertEquals(2, searcher.count(query));
+
+      // the index sort can't be used and the fallback has no cheap count
+      Weight weight = query.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1.0f);
+      for (LeafReaderContext context : searcher.getLeafContexts()) {
+        assertEquals(-1, weight.count(context));
+      }
+
+      writer.close();
+      reader.close();
+      dir.close();
+    }
+  }
+
+  /**
+   * With a parent field only the blocks are sorted, by the sort key of their parent, so the values
+   * by doc ID don't follow the sort order. Neither the doc values binary search nor the doc-ID
+   * range derived from the points can be used then.
+   */
+  public void testBlocksChildrenWithoutValue() throws IOException {

Review Comment:
   I think it's worth covering both cases to guard against future regressions.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Skip index-sort range shortcuts on segments with blocks and multi-valued fields [lucene]

Posted by "kkewwei (via GitHub)" <gi...@apache.org>.
kkewwei commented on code in PR #16893:
URL: https://github.com/apache/lucene/pull/16893#discussion_r4373961216


##########
lucene/core/src/test/org/apache/lucene/search/TestIndexSortSortedNumericDocValuesRangeQuery.java:
##########
@@ -551,6 +555,144 @@ public void testMultiDocValues() throws Exception {
     dir.close();
   }
 
+  /**
+   * Weight#count must not use the index sort when a document has multiple values: the index sort
+   * only reflects one value per document (the min by default), but the range query matches a
+   * document if any of its values is in range.
+   */
+  public void testCountWithMultiValuedField() throws IOException {
+    for (boolean withMissingDoc : new boolean[] {false, true}) {
+      Directory dir = newDirectory();
+      IndexWriterConfig iwc = new IndexWriterConfig(new MockAnalyzer(random()));
+      iwc.setIndexSort(new Sort(new SortedNumericSortField("field", SortField.Type.LONG)));
+      RandomIndexWriter writer = new RandomIndexWriter(random(), dir, iwc);
+
+      if (withMissingDoc) {
+        // sorts as the missing value 0, which is outside the query range
+        writer.addDocument(createMissingValueDocument());
+      }
+      // these sort by their min value (1, 2 and 60) but match [50, 200] through their max value
+      long[][] docs = {{1, 100}, {2, 3}, {60, 70}};
+      for (long[] values : docs) {
+        Document doc = new Document();
+        for (long value : values) {
+          doc.add(new SortedNumericDocValuesField("field", value));
+        }
+        writer.addDocument(doc);
+      }
+      writer.forceMerge(1);
+
+      IndexReader reader = writer.getReader();
+      IndexSearcher searcher = newSearcher(reader, false);
+
+      Query query = createQuery("field", 50, 200);
+      assertEquals(
+          2, searcher.count(SortedNumericDocValuesField.newSlowRangeQuery("field", 50, 200)));
+      assertEquals(2, searcher.count(query));
+
+      // the index sort can't be used and the fallback has no cheap count
+      Weight weight = query.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1.0f);
+      for (LeafReaderContext context : searcher.getLeafContexts()) {
+        assertEquals(-1, weight.count(context));
+      }
+
+      writer.close();
+      reader.close();
+      dir.close();
+    }
+  }
+
+  /**
+   * With a parent field only the blocks are sorted, by the sort key of their parent, so the values
+   * by doc ID don't follow the sort order. Neither the doc values binary search nor the doc-ID
+   * range derived from the points can be used then.
+   */
+  public void testBlocksChildrenWithoutValue() throws IOException {

Review Comment:
   Are the `testBlocksChildrenWithoutValue` and `testBlocksChildrenWithValue` covering the same case? Could we remove one of them?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Skip index-sort range shortcuts on segments with blocks and multi-valued fields [lucene]

Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman merged PR #16893:
URL: https://github.com/apache/lucene/pull/16893


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org