Skip to content

Commit 21ab39e

Browse files
intersectRight handle single child case
Signed-off-by: Prudhvi Godithi <[email protected]>
1 parent 6dad756 commit 21ab39e

File tree

2 files changed

+52
-10
lines changed

2 files changed

+52
-10
lines changed

server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,12 +301,18 @@ public void intersectRight(PointValues.IntersectVisitor visitor, PointValues.Poi
301301
PointValues.PointTree leftChild = pointTree.clone();
302302
// BKD is binary today, so one moveToSibling() is enough to land on the right child.
303303
// If PointTree ever becomes n-ary, update the traversal below to visit all siblings or re-enable a full loop.
304-
pointTree.moveToSibling();
305-
assert pointTree.moveToSibling() == false;
306-
intersectRight(visitor, pointTree, docCount);
307-
pointTree.moveToParent();
308-
if (docCount[0] < size) {
304+
if (pointTree.moveToSibling()) {
305+
// We have two children - visit right first
306+
intersectRight(visitor, pointTree, docCount);
307+
pointTree.moveToParent();
308+
// Then visit left if we still need more docs
309+
if (docCount[0] < size) {
310+
intersectRight(visitor, leftChild, docCount);
311+
}
312+
} else {
313+
// Only one child - visit it
309314
intersectRight(visitor, leftChild, docCount);
315+
pointTree.moveToParent();
310316
}
311317
} else {
312318
if (docCount[0] < size) {

server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,6 @@ public void testApproximateRangeShortCircuitDescSort() throws IOException {
395395
long[] scratch = new long[dims];
396396
int numPoints = 1000;
397397
for (int i = 0; i < numPoints; i++) {
398-
Document doc = new Document();
399398
for (int v = 0; v < dims; v++) {
400399
scratch[v] = i;
401400
}
@@ -487,7 +486,6 @@ public void testIntersectRightCellInsideQueryLeaf() throws IOException {
487486
long[] scratch = new long[dims];
488487
// Create a smaller dataset that will result in leaf nodes that are completely inside the query range
489488
for (int i = 900; i <= 999; i++) {
490-
Document doc = new Document();
491489
scratch[0] = i;
492490
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
493491
}
@@ -530,13 +528,11 @@ public void testIntersectRightCellOutsideQuery() throws IOException {
530528
// Create documents in two separate ranges to ensure some cells are outside query
531529
// Range 1: 0-99
532530
for (int i = 0; i < 100; i++) {
533-
Document doc = new Document();
534531
scratch[0] = i;
535532
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
536533
}
537534
// Range 2: 500-599 (gap ensures some tree nodes will be completely outside query)
538535
for (int i = 500; i < 600; i++) {
539-
Document doc = new Document();
540536
scratch[0] = i;
541537
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
542538
}
@@ -577,7 +573,6 @@ public void testIntersectRightCellCrossesQuery() throws IOException {
577573
long[] scratch = new long[dims];
578574
// Create documents that will result in cells that cross the query boundary
579575
for (int i = 0; i < 1000; i++) {
580-
Document doc = new Document();
581576
scratch[0] = i;
582577
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
583578
}
@@ -611,4 +606,45 @@ public void testIntersectRightCellCrossesQuery() throws IOException {
611606
}
612607
}
613608
}
609+
610+
// Test to specifically cover the single child case in intersectRight
611+
public void testIntersectRightSingleChildNode() throws IOException {
612+
try (Directory directory = newDirectory()) {
613+
try (RandomIndexWriter iw = new RandomIndexWriter(random(), directory, new WhitespaceAnalyzer())) {
614+
int dims = 1;
615+
long[] scratch = new long[dims];
616+
617+
for (int i = 0; i < 100; i++) {
618+
scratch[0] = 1000L;
619+
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
620+
}
621+
scratch[0] = 987654321L;
622+
iw.addDocument(asList(new LongPoint("point", scratch[0]), new NumericDocValuesField("point", scratch[0])));
623+
624+
iw.flush();
625+
iw.forceMerge(1);
626+
627+
try (IndexReader reader = iw.getReader()) {
628+
long lower = 500L;
629+
long upper = 999999999L;
630+
631+
ApproximatePointRangeQuery query = new ApproximatePointRangeQuery(
632+
"point",
633+
pack(lower).bytes,
634+
pack(upper).bytes,
635+
dims,
636+
50,
637+
SortOrder.DESC,
638+
ApproximatePointRangeQuery.LONG_FORMAT
639+
);
640+
641+
IndexSearcher searcher = new IndexSearcher(reader);
642+
Sort sort = new Sort(new SortField("point", SortField.Type.LONG, true));
643+
TopDocs topDocs = searcher.search(query, 50, sort);
644+
645+
assertEquals("Should return exactly 50 documents", 50, topDocs.scoreDocs.length);
646+
}
647+
}
648+
}
649+
}
614650
}

0 commit comments

Comments
 (0)