Skip to content

Commit d50ee28

Browse files
committed
Clearer error when vectorSearch() is used without a table alias
Running `SELECT ... FROM vectorSearch(...)` without an AS <alias> previously produced an opaque parser error from the legacy SQL engine fallback (`ERROR. token : TABLE, pos : 32`). Make the alias optional in the grammar rule for `tableFunctionRelation` and reject the missing-alias case in AstBuilder with a SemanticCheckException that tells the user to add an alias, for example `FROM vectorSearch(...) AS v`. SemanticCheckException (not SyntaxCheckException) is used on purpose so the request does not fall back to the legacy engine, whose opaque error would otherwise mask this message. Signed-off-by: Eric Wei <mengwei.eric@gmail.com>
1 parent 24bf6e0 commit d50ee28

4 files changed

Lines changed: 49 additions & 10 deletions

File tree

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,4 +434,25 @@ public void testMissingRequiredArgRejected() throws IOException {
434434

435435
assertThat(ex.getMessage(), containsString("requires 4 arguments"));
436436
}
437+
438+
/**
439+
* Users running FROM vectorSearch(...) without an AS alias previously received an opaque parser
440+
* error from the legacy SQL engine fallback. The clearer SemanticCheckException from the v2
441+
* engine must surface to the user instead.
442+
*/
443+
@Test
444+
public void testVectorSearchRequiresAlias() throws IOException {
445+
ResponseException ex =
446+
expectThrows(
447+
ResponseException.class,
448+
() ->
449+
executeQuery(
450+
"SELECT * FROM vectorSearch("
451+
+ "table='t', field='f', vector='[1.0]', option='k=5') "
452+
+ "LIMIT 3"));
453+
454+
String body = ex.getMessage();
455+
assertThat(body, containsString("requires a table alias"));
456+
assertThat(body, containsString("vectorSearch"));
457+
}
437458
}

sql/src/main/antlr/OpenSearchSQLParser.g4

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ fromClause
109109
;
110110

111111
relation
112-
: tableName (AS? alias)? # tableAsRelation
113-
| LR_BRACKET subquery = querySpecification RR_BRACKET AS? alias # subqueryAsRelation
114-
| qualifiedName LR_BRACKET tableFunctionArgs RR_BRACKET AS? alias # tableFunctionRelation
112+
: tableName (AS? alias)? # tableAsRelation
113+
| LR_BRACKET subquery = querySpecification RR_BRACKET AS? alias # subqueryAsRelation
114+
| qualifiedName LR_BRACKET tableFunctionArgs RR_BRACKET (AS? alias)? # tableFunctionRelation
115115
;
116116

117117
tableFunctionArgs

sql/src/main/java/org/opensearch/sql/sql/parser/AstBuilder.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,19 @@ public UnresolvedPlan visitTableFunctionRelation(TableFunctionRelationContext ct
228228
});
229229
TableFunction tableFunction =
230230
new TableFunction(visitAstExpression(ctx.qualifiedName()), args.build());
231+
if (ctx.alias() == null) {
232+
String functionName = ctx.qualifiedName().getText();
233+
// Use SemanticCheckException (not SyntaxCheckException) so the request does not fall back
234+
// to the legacy SQL engine, whose opaque parser error would mask this message.
235+
throw new SemanticCheckException(
236+
String.format(
237+
Locale.ROOT,
238+
"Table function '%s' requires a table alias."
239+
+ " Add an alias after the closing parenthesis, for example:"
240+
+ " FROM %s(...) AS v",
241+
functionName,
242+
functionName));
243+
}
231244
String alias = StringUtils.unquoteIdentifier(ctx.alias().getText());
232245
return new SubqueryAlias(alias, tableFunction);
233246
}

sql/src/test/java/org/opensearch/sql/sql/parser/AstBuilderTest.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package org.opensearch.sql.sql.parser;
77

88
import static java.util.Collections.emptyList;
9+
import static org.hamcrest.MatcherAssert.assertThat;
10+
import static org.hamcrest.Matchers.containsString;
911
import static org.junit.jupiter.api.Assertions.assertEquals;
1012
import static org.junit.jupiter.api.Assertions.assertThrows;
1113
import static org.opensearch.sql.ast.dsl.AstDSL.agg;
@@ -243,13 +245,16 @@ public void table_function_allows_alias_without_as_keyword() {
243245

244246
@Test
245247
public void table_function_relation_requires_alias() {
246-
assertThrows(
247-
SyntaxCheckException.class,
248-
() ->
249-
buildAST(
250-
"SELECT * FROM vectorSearch("
251-
+ "table='products', field='embedding', "
252-
+ "vector='[0.1,0.2]', option='k=10')"));
248+
SemanticCheckException ex =
249+
assertThrows(
250+
SemanticCheckException.class,
251+
() ->
252+
buildAST(
253+
"SELECT * FROM vectorSearch("
254+
+ "table='products', field='embedding', "
255+
+ "vector='[0.1,0.2]', option='k=10')"));
256+
assertThat(ex.getMessage(), containsString("requires a table alias"));
257+
assertThat(ex.getMessage(), containsString("vectorSearch"));
253258
}
254259

255260
@Test

0 commit comments

Comments
 (0)