Skip to content

Commit aad7f93

Browse files
committed
Address review nits: outer-bool guard, helper comment, efficient+ORDER BY
- Add assertFalse(contains("\"bool\"")) on the outer sourceBuilder JSON for the plain top-k and radial (max_distance, min_score) cases so a regression that wrapped the knn in an outer bool while preserving the inner payload is caught. - Document the SOURCE_BUILDER_JSON helper's test-only coupling to the surrounding "sourceBuilder=...", "pitId=" tokens in the request-string toString() output — if that format changes, update the regex anchors. - Strengthen testEfficientFilterWithOrderByScoreDescSucceeds onto the same helpers as testExplainFilterTypeEfficientProducesKnnWithFilter: verify no outer bool/must in the sourceBuilder JSON and that the WHERE predicate is embedded inside the decoded knn payload. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 0ad6d2a commit aad7f93

1 file changed

Lines changed: 39 additions & 2 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchExplainIT.java

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ public class VectorSearchExplainIT extends SQLIntegTestCase {
2727
// quotes as \", so the regex tolerates both \" and " forms around the query key/value.
2828
private static final Pattern WRAPPER_PAYLOAD =
2929
Pattern.compile("\\\\?\"query\\\\?\":\\\\?\"([A-Za-z0-9+/=]+)\\\\?\"");
30+
// Anchored on the surrounding `sourceBuilder=...`, `pitId=` tokens in OpenSearchRequest's
31+
// toString() output. Test-only coupling: if that request-string format changes (token renamed,
32+
// pitId removed), this helper breaks even when the DSL shape is still correct. Update the regex
33+
// anchors if that happens.
3034
private static final Pattern SOURCE_BUILDER_JSON =
3135
Pattern.compile("sourceBuilder=(\\{.*?\\}), pitId=", Pattern.DOTALL);
3236

@@ -79,6 +83,12 @@ public void testExplainTopKProducesKnnQuery() throws IOException {
7983
assertTrue(
8084
"Explain should contain track_scores:\n" + explain, explain.contains("track_scores"));
8185

86+
// Top-k without WHERE should have the knn at the root, not wrapped in an outer bool.
87+
String sourceBuilderJson = extractSourceBuilderJson(explain);
88+
assertFalse(
89+
"Top-k without WHERE should not wrap knn in an outer bool:\n" + sourceBuilderJson,
90+
sourceBuilderJson.contains("\"bool\""));
91+
8292
String knnJson = decodeSoleKnnJson(explain);
8393
assertTrue("knn JSON should contain knn key:\n" + knnJson, knnJson.contains("\"knn\""));
8494
assertTrue(
@@ -103,6 +113,12 @@ public void testExplainRadialMaxDistanceProducesKnnQuery() throws IOException {
103113
+ "vector='[1.0, 2.0]', option='max_distance=10.5') AS v "
104114
+ "LIMIT 100");
105115

116+
// Radial without WHERE should have the knn at the root, not wrapped in an outer bool.
117+
String sourceBuilderJson = extractSourceBuilderJson(explain);
118+
assertFalse(
119+
"Radial without WHERE should not wrap knn in an outer bool:\n" + sourceBuilderJson,
120+
sourceBuilderJson.contains("\"bool\""));
121+
106122
String knnJson = decodeSoleKnnJson(explain);
107123
assertTrue("knn JSON should contain knn key:\n" + knnJson, knnJson.contains("\"knn\""));
108124
assertTrue(
@@ -128,6 +144,12 @@ public void testExplainRadialMinScoreProducesKnnQuery() throws IOException {
128144
+ "vector='[1.0, 2.0]', option='min_score=0.8') AS v "
129145
+ "LIMIT 100");
130146

147+
// Radial without WHERE should have the knn at the root, not wrapped in an outer bool.
148+
String sourceBuilderJson = extractSourceBuilderJson(explain);
149+
assertFalse(
150+
"Radial without WHERE should not wrap knn in an outer bool:\n" + sourceBuilderJson,
151+
sourceBuilderJson.contains("\"bool\""));
152+
131153
String knnJson = decodeSoleKnnJson(explain);
132154
assertTrue("knn JSON should contain knn key:\n" + knnJson, knnJson.contains("\"knn\""));
133155
assertTrue(
@@ -406,8 +428,23 @@ public void testEfficientFilterWithOrderByScoreDescSucceeds() throws IOException
406428
+ "ORDER BY v._score DESC "
407429
+ "LIMIT 5");
408430

431+
// Same efficient-mode shape guarantee as testExplainFilterTypeEfficientProducesKnnWithFilter,
432+
// with an added ORDER BY _score DESC: no outer bool/must, and the WHERE predicate must be
433+
// embedded inside the knn payload (efficient filtering, not post-filter).
434+
String sourceBuilderJson = extractSourceBuilderJson(explain);
435+
assertFalse(
436+
"Efficient mode should not produce bool query (that is post-filter shape):\n"
437+
+ sourceBuilderJson,
438+
sourceBuilderJson.contains("\"bool\""));
439+
assertFalse(
440+
"Efficient mode should not contain must clause:\n" + sourceBuilderJson,
441+
sourceBuilderJson.contains("\"must\""));
442+
443+
String knnJson = decodeSoleKnnJson(explain);
409444
assertTrue(
410-
"Explain should succeed with efficient + ORDER BY _score DESC:\n" + explain,
411-
explain.contains("wrapper"));
445+
"Efficient mode knn JSON should contain filter:\n" + knnJson, knnJson.contains("filter"));
446+
assertTrue(
447+
"Efficient mode knn JSON should contain the WHERE predicate field:\n" + knnJson,
448+
knnJson.contains("state"));
412449
}
413450
}

0 commit comments

Comments
 (0)