Skip to content

Commit 08ab0c6

Browse files
committed
Preserve sort.getCount() limit pushdown contract in pushDownSort
The base OpenSearchIndexScanQueryBuilder.pushDownSort() pushes sort.getCount() as a limit when non-zero. Our override validated _score DESC and returned true, but did not preserve this contract. SQL always sets count=0, so this was not reachable today, but PPL or future callers may set a non-zero count to combine sort+limit in one LogicalSort node. Preserve the behavior defensively. Add focused test: LogicalSort(count=7) with _score DESC verifies the count is pushed down as request size. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 12709e4 commit 08ab0c6

2 files changed

Lines changed: 31 additions & 1 deletion

File tree

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilder.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ public boolean pushDownSort(LogicalSort sort) {
8686
"vectorSearch only supports ORDER BY _score DESC; _score ASC is not supported");
8787
}
8888
}
89-
// _score DESC is the natural knn order — accept without pushing sort to OpenSearch
89+
// _score DESC is the natural knn order — no need to push the sort itself to OpenSearch.
90+
// Preserve the parent's sort.getCount() → limit pushdown contract: SQL always sets count=0,
91+
// but PPL or future callers may set a non-zero count to combine sort+limit in one node.
92+
if (sort.getCount() != 0) {
93+
requestBuilder.pushDownLimit(sort.getCount(), 0);
94+
}
9095
return true;
9196
}
9297
}

opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilderTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,31 @@ void pushDownSortScoreDescAccepted() {
155155
assertTrue(pushed, "ORDER BY _score DESC should be accepted");
156156
}
157157

158+
@Test
159+
void pushDownSortPreservesSortCountAsLimit() {
160+
var requestBuilder = createRequestBuilder();
161+
var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
162+
var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "10"));
163+
164+
var dummyChild = new LogicalValues(Collections.emptyList());
165+
// LogicalSort with count=7 simulates a sort+limit combined node (PPL path)
166+
var sort =
167+
new org.opensearch.sql.planner.logical.LogicalSort(
168+
dummyChild,
169+
7,
170+
List.of(
171+
org.apache.commons.lang3.tuple.ImmutablePair.of(
172+
org.opensearch.sql.ast.tree.Sort.SortOption.DEFAULT_DESC,
173+
new ReferenceExpression("_score", ExprCoreType.FLOAT))));
174+
175+
boolean pushed = builder.pushDownSort(sort);
176+
assertTrue(pushed, "ORDER BY _score DESC with count should be accepted");
177+
assertEquals(
178+
7,
179+
requestBuilder.getMaxResponseSize(),
180+
"sort.getCount() should be pushed down as request size");
181+
}
182+
158183
@Test
159184
void pushDownSortNonScoreFieldRejected() {
160185
var requestBuilder = createRequestBuilder();

0 commit comments

Comments
 (0)