Skip to content

Commit 94f1691

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 fa444fe commit 94f1691

4 files changed

Lines changed: 51 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
@@ -300,4 +300,25 @@ public void testExplainWithoutKnnPluginStillWorks() throws IOException {
300300

301301
assertThat(explain, containsString("wrapper"));
302302
}
303+
304+
/**
305+
* Users running FROM vectorSearch(...) without an AS alias previously received an opaque parser
306+
* error from the legacy SQL engine fallback. The clearer SemanticCheckException from the v2
307+
* engine must surface to the user instead.
308+
*/
309+
@Test
310+
public void testVectorSearchRequiresAlias() throws IOException {
311+
ResponseException ex =
312+
expectThrows(
313+
ResponseException.class,
314+
() ->
315+
executeQuery(
316+
"SELECT * FROM vectorSearch("
317+
+ "table='t', field='f', vector='[1.0]', option='k=5') "
318+
+ "LIMIT 3"));
319+
320+
String body = ex.getMessage();
321+
assertThat(body, containsString("requires a table alias"));
322+
assertThat(body, containsString("vectorSearch"));
323+
}
303324
}

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: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.opensearch.sql.ast.tree.Values;
4343
import org.opensearch.sql.common.antlr.SyntaxCheckException;
4444
import org.opensearch.sql.common.utils.StringUtils;
45+
import org.opensearch.sql.exception.SemanticCheckException;
4546
import org.opensearch.sql.expression.function.BuiltinFunctionName;
4647
import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser;
4748
import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.QuerySpecificationContext;
@@ -207,6 +208,19 @@ public UnresolvedPlan visitTableFunctionRelation(TableFunctionRelationContext ct
207208
});
208209
TableFunction tableFunction =
209210
new TableFunction(visitAstExpression(ctx.qualifiedName()), args.build());
211+
if (ctx.alias() == null) {
212+
String functionName = ctx.qualifiedName().getText();
213+
// Use SemanticCheckException (not SyntaxCheckException) so the request does not fall back
214+
// to the legacy SQL engine, whose opaque parser error would mask this message.
215+
throw new SemanticCheckException(
216+
String.format(
217+
Locale.ROOT,
218+
"Table function '%s' requires a table alias."
219+
+ " Add an alias after the closing parenthesis, for example:"
220+
+ " FROM %s(...) AS v",
221+
functionName,
222+
functionName));
223+
}
210224
String alias = StringUtils.unquoteIdentifier(ctx.alias().getText());
211225
return new SubqueryAlias(alias, tableFunction);
212226
}

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

Lines changed: 13 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;
@@ -44,6 +46,7 @@
4446
import org.opensearch.sql.ast.tree.SubqueryAlias;
4547
import org.opensearch.sql.ast.tree.TableFunction;
4648
import org.opensearch.sql.common.antlr.SyntaxCheckException;
49+
import org.opensearch.sql.exception.SemanticCheckException;
4750

4851
class AstBuilderTest extends AstBuilderTestBase {
4952

@@ -242,13 +245,16 @@ public void table_function_allows_alias_without_as_keyword() {
242245

243246
@Test
244247
public void table_function_relation_requires_alias() {
245-
assertThrows(
246-
SyntaxCheckException.class,
247-
() ->
248-
buildAST(
249-
"SELECT * FROM vectorSearch("
250-
+ "table='products', field='embedding', "
251-
+ "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"));
252258
}
253259

254260
@Test

0 commit comments

Comments
 (0)