[Feature] Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch() by mengweieric · Pull Request #5324 · opensearch-project/sql

mengweieric · 2026-04-08T23:32:01Z

Summary

Adds validation hardening, query restrictions, and integration tests on top of the
core vectorSearch() execution pipeline in #5320.

What this PR adds

Validation hardening

Option mutual exclusivity: exactly one of k, max_distance, min_score
k range validation: [1, 10000]
LIMIT > k rejection in top-k mode
Sort restriction: only ORDER BY _score DESC allowed; rejects non-_score
fields and _score ASC
Radial size policy: caps max_distance/min_score queries at maxResultWindow
to prevent unbounded result sets

pushDownSort() contract preservation

Preserves base OpenSearchIndexScanQueryBuilder.pushDownSort() contract for
sort.getCount() → limit pushdown. SQL always sets count=0, but PPL or future
callers may combine sort+limit in one LogicalSort node. Focused test verifies
LogicalSort(count=7) is correctly pushed down as request size.

Test coverage

VectorSearchIndexTest (9 tests): knn JSON generation, options rendering,
non-numeric option quoting, nested fields
VectorSearchIT (16 integration tests): 4 _explain DSL shape verification +
8 validation error tests + 3 ORDER BY restriction tests + 1 LIMIT success test
Additional unit tests across existing test files: multi-sort rejection,
case-insensitive arg lookup, sort count preservation, radial LIMIT modes,
resolver argument count edge cases, NaN/Infinity edge cases

Benchmark results (against live OpenSearch + k-NN)

Semantic correctness: 10/10 on all 8 test cases (top-k, radial, post-filter, hotels)
SQL overhead: +1.4ms to +2.6ms p50 vs native DSL

Test plan

./gradlew spotlessCheck — PASS
./gradlew :opensearch:test :core:test :sql:test — PASS
./gradlew :integ-test:integTest -Dtests.class="*VectorSearchIT" — 16/16 PASS
Correctness benchmark: 10/10 semantic match across A1, A2, A4, A5, A6a, A6b, A7-tight, A8

- Enforce exactly one of k, max_distance, or min_score - Validate k is in [1, 10000] range - Add 6 tests: mutual exclusivity (3 combos), k too small, k too large, k boundary values (1 and 10000) Signed-off-by: Eric Wei <[email protected]>

VectorSearchQueryBuilder now accepts options map and rejects pushDownLimit when LIMIT exceeds k. Radial modes (max_distance, min_score) have no LIMIT restriction. Signed-off-by: Eric Wei <[email protected]>

- Create VectorSearchIndexTest: 7 tests covering buildKnnQueryJson() for top-k, max_distance, min_score, nested fields, multi-element and single-element vectors, numeric option rendering - Add edge case tests to VectorSearchTableFunctionImplementationTest: NaN vector component, empty option key/value, negative k, NaN for max_distance and min_score (6 new tests) - Add VectorSearchQueryBuilderTest: min_score radial mode LIMIT, pushDownSort delegation to parent (2 new tests) - Extract buildKnnQueryJson() as package-private for direct testing Signed-off-by: Eric Wei <[email protected]>

Test too-many (5) and zero arguments paths in VectorSearchTableFunctionResolver to complement existing too-few (2) test. Signed-off-by: Eric Wei <[email protected]>

- Cap radial mode (max_distance/min_score) results at maxResultWindow to prevent unbounded result sets - Reject ORDER BY on non-_score fields and _score ASC in vectorSearch since knn results are naturally sorted by _score DESC - Add 12 integration tests: 4 _explain DSL shape verification tests and 8 validation error path tests Signed-off-by: Eric Wei <[email protected]>

- Add multi-sort expression test: ORDER BY _score DESC, name ASC correctly rejects the non-_score field (VectorSearchQueryBuilderTest) - Add case-insensitive argument name lookup test to verify TABLE='x' resolves same as table='x' (Implementation test) - Add non-numeric option fallback test: verifies string options are quoted in JSON output (VectorSearchIndexTest) - Add 4 integration tests: ORDER BY _score DESC succeeds, ORDER BY non-score rejects, ORDER BY _score ASC rejects, LIMIT within k succeeds (VectorSearchIT, now 16 tests) Signed-off-by: Eric Wei <[email protected]>

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 <[email protected]>

mengweieric · 2026-04-09T21:25:03Z

    if (hasK) {
      parseIntOption(options, "k");
+      int k = Integer.parseInt(options.get("k"));
+      if (k < 1 || k > 10000) {


source of k in [1, 10,000]: https://docs.opensearch.org/latest/vector-search/vector-search-techniques/approximate-knn/#:~:text=The%20maximum%20value%20of%20k%20is%2010%2C000.

- Unit test: compound AND predicate survives pushdown into bool.filter - Integration test: compound WHERE (term + range) produces bool query - Integration test: radial max_distance with WHERE produces bool query Signed-off-by: Eric Wei <[email protected]>

RyanL1997

Hi @mengweieric , thanks for the change and I just left a quesiton.

RyanL1997 · 2026-04-13T21:32:12Z

+    // Preserve the parent's sort.getCount() → limit pushdown contract: SQL always sets count=0,
+    // but PPL or future callers may set a non-zero count to combine sort+limit in one node.
+    if (sort.getCount() != 0) {
+      requestBuilder.pushDownLimit(sort.getCount(), 0);


nit:

pushDownSort calls requestBuilder.pushDownLimit(sort.getCount(), 0) directly, bypassing this.pushDownLimit() (L59-67) which has the LIMIT > k validation.

I understand vectorSearch is SQL-only for P0, and in the SQL path sort.getCount() is always 0 so this is a no-op today. But the comment on L90-91 explicitly anticipates the PPL path ("PPL or future callers may set a non-zero count") — so just want to confirm: if/when PPL does use this path with a non-zero count, should the LIMIT > k guard still apply? Or is it intentionally unchecked here?

Good catch. The sort.getCount() path was not preserving the same LIMIT <= k invariant as pushDownLimit(). I fixed that by routing both paths through the same validation helper and added tests for the sort-count case. Thanks for calling it out.

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 <[email protected]>

ahkcs · 2026-04-14T20:45:15Z

+
+  @Test
+  public void testExplainTopKProducesKnnQuery() throws IOException {
+    String explain =


Can we move the Explain test to a separate ExplainIT suite so that we can view the results in a yaml or json file directly
ref: CalciteExplainIT

Thanks, done. The _explain cases now live in VectorSearchExplainIT, and VectorSearchIT keeps only behavior/validation tests. I kept the current SQL explain assertions for now; happy to follow up with file-backed expected outputs if we want to standardize that too.

Move all explainQuery()-based DSL shape tests into a dedicated VectorSearchExplainIT suite. VectorSearchIT now contains only validation and error-path tests. Signed-off-by: Eric Wei <[email protected]>

mengweieric added SQL feature skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels Apr 8, 2026

mengweieric requested review from LantaoJin, RyanL1997, Swiddis, acarbonetto, ahkcs, anirudha, dai-chen, joshuali925, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners April 8, 2026 23:32

mengweieric changed the title ~~Add vectorSearch execution pipeline, validation hardening, and integration tests~~ [Feature] Add vectorSearch execution pipeline, validation hardening, and integration tests Apr 8, 2026

mengweieric changed the title ~~[Feature] Add vectorSearch execution pipeline, validation hardening, and integration tests~~ [Feature] Add vectorSearch validation hardening, and integration tests Apr 8, 2026

mengweieric changed the title ~~[Feature] Add vectorSearch validation hardening, and integration tests~~ Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch() Apr 8, 2026

mengweieric changed the title ~~Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch()~~ [Feature] Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch() Apr 8, 2026

mengweieric added 6 commits April 9, 2026 11:45

Add mutual exclusivity and k range validation

9ff5d2d

- Enforce exactly one of k, max_distance, or min_score - Validate k is in [1, 10000] range - Add 6 tests: mutual exclusivity (3 combos), k too small, k too large, k boundary values (1 and 10000) Signed-off-by: Eric Wei <[email protected]>

Add LIMIT > k rejection in top-k vector search mode

7f8ed14

VectorSearchQueryBuilder now accepts options map and rejects pushDownLimit when LIMIT exceeds k. Radial modes (max_distance, min_score) have no LIMIT restriction. Signed-off-by: Eric Wei <[email protected]>

Add resolver argument count edge case tests

767c7f0

Test too-many (5) and zero arguments paths in VectorSearchTableFunctionResolver to complement existing too-few (2) test. Signed-off-by: Eric Wei <[email protected]>

mengweieric force-pushed the feature/vector-search-p0-hardening branch from 08ab0c6 to 534b5f4 Compare April 9, 2026 18:46

mengweieric commented Apr 9, 2026

View reviewed changes

mengweieric force-pushed the feature/vector-search-p0-hardening branch from 5b0373c to 0e02488 Compare April 9, 2026 22:36

RyanL1997 reviewed Apr 13, 2026

View reviewed changes

ahkcs reviewed Apr 14, 2026

View reviewed changes

Comment thread integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchIT.java Outdated

Split explain tests into dedicated VectorSearchExplainIT

4a744d4

Move all explainQuery()-based DSL shape tests into a dedicated VectorSearchExplainIT suite. VectorSearchIT now contains only validation and error-path tests. Signed-off-by: Eric Wei <[email protected]>

ahkcs approved these changes Apr 14, 2026

View reviewed changes

RyanL1997 approved these changes Apr 14, 2026

View reviewed changes

mengweieric merged commit b7b37d1 into opensearch-project:feature/vector-search-p0 Apr 14, 2026
36 of 37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch()#5324

[Feature] Add validation hardening, sort/limit restrictions, and integration tests for vectorSearch()#5324
mengweieric merged 10 commits intoopensearch-project:feature/vector-search-p0from
mengweieric:feature/vector-search-p0-hardening

mengweieric commented Apr 8, 2026 •

edited

Loading

Uh oh!

mengweieric Apr 9, 2026

Uh oh!

RyanL1997 left a comment

Uh oh!

RyanL1997 Apr 13, 2026 •

edited

Loading

Uh oh!

mengweieric Apr 14, 2026

Uh oh!

ahkcs Apr 14, 2026

Uh oh!

mengweieric Apr 14, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mengweieric commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this PR adds

Test plan

Uh oh!

mengweieric Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

RyanL1997 left a comment

Choose a reason for hiding this comment

Uh oh!

RyanL1997 Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mengweieric Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ahkcs Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

mengweieric Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mengweieric commented Apr 8, 2026 •

edited

Loading

RyanL1997 Apr 13, 2026 •

edited

Loading