Skip to content

Commit 8af0fec

Browse files
committed
Address PR review: operator-specific error messages, inner ORDER BY _score IT, reword pushdown phrasing
Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 8ecfdd3 commit 8af0fec

2 files changed

Lines changed: 49 additions & 13 deletions

File tree

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,24 @@ public void testSubqueryNoWhereStillWorks() throws IOException {
166166
assertThat(explain, containsString("wrapper"));
167167
}
168168

169+
@Test
170+
public void testInnerOrderByScoreDescInSubqueryAllowed() throws IOException {
171+
// Positive control: inner ORDER BY _score DESC on the vectorSearch() relation inside the
172+
// subquery is the only supported sort, and must continue to plan successfully even when
173+
// wrapped in an outer SELECT. Proves the walker does not over-reject sort shapes that are
174+
// below the subquery Project rather than above it.
175+
String explain =
176+
explainQuery(
177+
"SELECT * FROM (SELECT v.firstname, v._score "
178+
+ "FROM vectorSearch(table='"
179+
+ TEST_INDEX
180+
+ "', field='embedding', vector='[1.0, 2.0]', option='k=5') AS v "
181+
+ "ORDER BY v._score DESC) t "
182+
+ "LIMIT 3");
183+
184+
assertThat(explain, containsString("wrapper"));
185+
}
186+
169187
@Test
170188
public void testOuterOrderByOnSubqueryRejected() throws IOException {
171189
// Outer ORDER BY over a vectorSearch() subquery would run on a truncated top-k slice rather

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

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
* <li><b>Outer operators over a vectorSearch() subquery</b> — when vectorSearch() is wrapped in a
2828
* subquery (e.g. {@code SELECT * FROM (SELECT v.id FROM vectorSearch(...) AS v) t WHERE
2929
* t.price < 150}), outer WHERE / ORDER BY / OFFSET / GROUP BY / aggregation / DISTINCT do not
30-
* reach the push-down contract (the inner {@link LogicalProject} sits between the outer
31-
* operator and this scan builder, so those nodes never match the direct-adjacency push-down
32-
* patterns). They would then be applied in memory <i>after</i> k-NN has already returned
33-
* top-k documents ranked by vector distance, which can silently yield zero rows or
30+
* participate in the vectorSearch pushdown contract (the inner {@link LogicalProject} sits
31+
* between the outer operator and this scan builder, so those nodes never match the
32+
* direct-adjacency push-down patterns). They would then be applied in memory <i>after</i>
33+
* top-k results have been selected by vector distance, which can silently yield zero rows or
3434
* mis-ordered results. We detect these shapes in {@link #validatePlan(LogicalPlan)} and
3535
* reject with a clear error.
3636
* </ul>
@@ -128,15 +128,33 @@ private static String classifyOuterOperator(LogicalPlan node) {
128128
return null;
129129
}
130130

131+
// Operator-specific messages: the generic "move it inside the subquery" advice is only right
132+
// for WHERE and for ORDER BY _score DESC. OFFSET, aggregation, GROUP BY, and DISTINCT are
133+
// themselves unsupported on vectorSearch() directly, so the message must not claim a workaround
134+
// that would only trip the user on a second validation error.
131135
private static String rejectionMessage(String outerOp) {
132-
return "Outer "
133-
+ outerOp
134-
+ " on a vectorSearch() subquery is not supported: the operator does not push into the"
135-
+ " k-NN search and would be applied only after top-k results have been selected by"
136-
+ " vector distance, which can silently yield zero rows or mis-ordered results."
137-
+ " Move the "
138-
+ outerOp
139-
+ " inside the subquery (directly on vectorSearch()) so it can participate in the"
140-
+ " vectorSearch push-down contract.";
136+
switch (outerOp) {
137+
case "WHERE":
138+
return "Outer WHERE on a vectorSearch() subquery is not supported: the predicate does not"
139+
+ " participate in the vectorSearch pushdown contract and would be applied only"
140+
+ " after top-k results have been selected by vector distance, which can silently"
141+
+ " yield zero rows. Move the WHERE into the same SELECT block as vectorSearch() so"
142+
+ " it participates in the vectorSearch WHERE pushdown contract.";
143+
case "ORDER BY":
144+
return "Outer ORDER BY on a vectorSearch() subquery is not supported: sorting does not"
145+
+ " participate in the vectorSearch pushdown contract and would be applied only"
146+
+ " after top-k results have been selected by vector distance, which can yield"
147+
+ " mis-ordered results. Use ORDER BY <alias>._score DESC in the same SELECT block"
148+
+ " as vectorSearch(), or omit ORDER BY.";
149+
case "OFFSET":
150+
return "Outer OFFSET on a vectorSearch() subquery is not supported. OFFSET is not"
151+
+ " supported on vectorSearch(); use LIMIT only.";
152+
case "GROUP BY / aggregation / DISTINCT":
153+
return "Outer GROUP BY / aggregation / DISTINCT on a vectorSearch() subquery is not"
154+
+ " supported. Aggregations and DISTINCT are not supported on vectorSearch()"
155+
+ " relations.";
156+
default:
157+
return "Outer " + outerOp + " on a vectorSearch() subquery is not supported.";
158+
}
141159
}
142160
}

0 commit comments

Comments
 (0)