Skip to content

Commit 1cf35ce

Browse files
committed
Route pushDownSort count through LIMIT > k validation
pushDownSort() called requestBuilder.pushDownLimit() directly, bypassing the LIMIT > k guard in pushDownLimit(). Extract validateLimitWithinK() helper and call it from both paths so the invariant holds when PPL or future callers set a non-zero sort count. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 0e02488 commit 1cf35ce

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,7 @@ public boolean pushDownFilter(LogicalFilter filter) {
5757

5858
@Override
5959
public boolean pushDownLimit(LogicalLimit limit) {
60-
if (options.containsKey("k")) {
61-
int k = Integer.parseInt(options.get("k"));
62-
if (limit.getLimit() > k) {
63-
throw new ExpressionEvaluationException(
64-
String.format("LIMIT %d exceeds k=%d in top-k vector search", limit.getLimit(), k));
65-
}
66-
}
60+
validateLimitWithinK(limit.getLimit());
6761
return super.pushDownLimit(limit);
6862
}
6963

@@ -90,8 +84,20 @@ public boolean pushDownSort(LogicalSort sort) {
9084
// Preserve the parent's sort.getCount() → limit pushdown contract: SQL always sets count=0,
9185
// but PPL or future callers may set a non-zero count to combine sort+limit in one node.
9286
if (sort.getCount() != 0) {
87+
validateLimitWithinK(sort.getCount());
9388
requestBuilder.pushDownLimit(sort.getCount(), 0);
9489
}
9590
return true;
9691
}
92+
93+
/** Validates that the requested limit does not exceed k in top-k mode. */
94+
private void validateLimitWithinK(int limit) {
95+
if (options.containsKey("k")) {
96+
int k = Integer.parseInt(options.get("k"));
97+
if (limit > k) {
98+
throw new ExpressionEvaluationException(
99+
String.format("LIMIT %d exceeds k=%d in top-k vector search", limit, k));
100+
}
101+
}
102+
}
97103
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,28 @@ void pushDownSortPreservesSortCountAsLimit() {
180180
"sort.getCount() should be pushed down as request size");
181181
}
182182

183+
@Test
184+
void pushDownSortCountExceedingKRejects() {
185+
var requestBuilder = createRequestBuilder();
186+
var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
187+
var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
188+
189+
var dummyChild = new LogicalValues(Collections.emptyList());
190+
// LogicalSort with count=10 exceeds k=5 — should be rejected
191+
var sort =
192+
new org.opensearch.sql.planner.logical.LogicalSort(
193+
dummyChild,
194+
10,
195+
List.of(
196+
org.apache.commons.lang3.tuple.ImmutablePair.of(
197+
org.opensearch.sql.ast.tree.Sort.SortOption.DEFAULT_DESC,
198+
new ReferenceExpression("_score", ExprCoreType.FLOAT))));
199+
200+
ExpressionEvaluationException ex =
201+
assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownSort(sort));
202+
assertTrue(ex.getMessage().contains("LIMIT 10 exceeds k=5"));
203+
}
204+
183205
@Test
184206
void pushDownSortNonScoreFieldRejected() {
185207
var requestBuilder = createRequestBuilder();

0 commit comments

Comments
 (0)