Skip to content

Commit abb3ca4

Browse files
dai-chendai-chen
committed
feat(sql): support JOIN in unified query path (opensearch-project#5446)
Extend the V2 ANTLR grammar with JOIN clause rules (INNER/LEFT/RIGHT/ CROSS) and add ExtendedAstBuilder in SqlV2QueryParser to produce Join AST nodes for the Calcite-based unified query path. The base AstBuilder throws SyntaxCheckException to preserve legacy engine fallback. A semantic predicate prevents LEFT/RIGHT from being consumed as implicit table aliases when followed by JOIN. Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent 446f2ab commit abb3ca4

6 files changed

Lines changed: 129 additions & 47 deletions

File tree

‎api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ public static class Builder {
122122
* Setting values with defaults from SysLimit.DEFAULT. Only includes planning-required settings
123123
* to avoid coupling with OpenSearchSettings.
124124
*
125-
125+
* <p>{@link Settings.Key#PPL_JOIN_SUBSEARCH_MAXOUT} defaults to {@code 0} to avoid injecting
126+
* {@code LogicalSystemLimit} into the logical plan, which is an OpenSearch-specific operational
127+
* concern irrelevant to external consumers of the unified query API. {@link
128+
* Settings.Key#PPL_SUBSEARCH_MAXOUT} is set to {@code 0} for the same reason.
129+
*
126130
* <p>{@link Settings.Key#CALCITE_ENGINE_ENABLED} defaults to {@code true} here because the
127131
* unified query path is by definition Calcite-based — every query reaching this context flows
128132
* through Calcite's planner, never the v2 engine. The PPL {@link
@@ -143,8 +147,8 @@ public static class Builder {
143147
new HashMap<Settings.Key, Object>(
144148
Map.of(
145149
QUERY_SIZE_LIMIT, SysLimit.DEFAULT.querySizeLimit(),
146-
PPL_SUBSEARCH_MAXOUT, SysLimit.DEFAULT.subsearchLimit(),
147-
PPL_JOIN_SUBSEARCH_MAXOUT, SysLimit.DEFAULT.joinSubsearchLimit(),
150+
PPL_SUBSEARCH_MAXOUT, SysLimit.UNLIMITED_SUBSEARCH.subsearchLimit(),
151+
PPL_JOIN_SUBSEARCH_MAXOUT, SysLimit.UNLIMITED_SUBSEARCH.joinSubsearchLimit(),
148152
CALCITE_ENGINE_ENABLED, true,
149153
PPL_REX_MAX_MATCH_LIMIT, 10));
150154

‎api/src/main/java/org/opensearch/sql/api/parser/SqlV2QueryParser.java‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@
55

66
package org.opensearch.sql.api.parser;
77

8+
import static org.opensearch.sql.ast.dsl.AstDSL.join;
9+
10+
import java.util.Optional;
811
import org.antlr.v4.runtime.tree.ParseTree;
12+
import org.opensearch.sql.ast.expression.UnresolvedExpression;
913
import org.opensearch.sql.ast.statement.Query;
1014
import org.opensearch.sql.ast.statement.Statement;
15+
import org.opensearch.sql.ast.tree.Join.JoinType;
1116
import org.opensearch.sql.ast.tree.UnresolvedPlan;
1217
import org.opensearch.sql.sql.antlr.SQLSyntaxParser;
18+
import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser;
19+
import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.JoinClauseContext;
1320
import org.opensearch.sql.sql.parser.AstBuilder;
1421
import org.opensearch.sql.sql.parser.AstStatementBuilder;
1522

@@ -24,7 +31,8 @@ public UnresolvedPlan parse(String query) {
2431
ParseTree cst = syntaxParser.parse(query);
2532
AstStatementBuilder astStmtBuilder =
2633
new AstStatementBuilder(
27-
new AstBuilder(query), AstStatementBuilder.StatementBuilderContext.builder().build());
34+
new ExtendedAstBuilder(query),
35+
AstStatementBuilder.StatementBuilderContext.builder().build());
2836
Statement statement = cst.accept(astStmtBuilder);
2937

3038
if (statement instanceof Query) {
@@ -33,4 +41,33 @@ public UnresolvedPlan parse(String query) {
3341
throw new UnsupportedOperationException(
3442
"Only query statements are supported but got " + statement.getClass().getSimpleName());
3543
}
44+
45+
/**
46+
* Extends the V2 AstBuilder with JOIN support that the base AstBuilder rejects with
47+
* SyntaxCheckException to trigger legacy engine fallback.
48+
*/
49+
private static class ExtendedAstBuilder extends AstBuilder {
50+
51+
ExtendedAstBuilder(String query) {
52+
super(query);
53+
}
54+
55+
@Override
56+
public UnresolvedPlan visitJoinClause(JoinClauseContext ctx) {
57+
JoinType joinType = toJoinType(ctx);
58+
UnresolvedPlan right = visit(ctx.relation());
59+
Optional<UnresolvedExpression> condition =
60+
Optional.ofNullable(ctx.expression()).map(this::visitAstExpression);
61+
return join(right, joinType, condition);
62+
}
63+
64+
private JoinType toJoinType(JoinClauseContext ctx) {
65+
return switch (ctx.getStart().getType()) {
66+
case OpenSearchSQLParser.LEFT -> JoinType.LEFT;
67+
case OpenSearchSQLParser.RIGHT -> JoinType.RIGHT;
68+
case OpenSearchSQLParser.CROSS -> JoinType.CROSS;
69+
default -> JoinType.INNER;
70+
};
71+
}
72+
}
3673
}

‎api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java‎

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55

66
package org.opensearch.sql.api;
77

8+
import static org.apache.calcite.sql.type.SqlTypeName.INTEGER;
9+
import static org.apache.calcite.sql.type.SqlTypeName.VARCHAR;
10+
11+
import java.util.Map;
12+
import org.apache.calcite.schema.Table;
13+
import org.apache.calcite.schema.impl.AbstractSchema;
14+
import org.junit.Before;
815
import org.junit.Test;
916
import org.opensearch.sql.executor.QueryType;
1017

1118
/**
12-
* Tests for basic SQL query planning through the V2 ANTLR parser path. Covers SELECT, WHERE, ORDER
13-
* BY operations that produce valid RelNode plans.
19+
* Tests for SQL query planning through the V2 ANTLR parser path. Covers SELECT, WHERE, ORDER BY,
20+
* and JOIN operations that produce valid RelNode plans.
1421
*/
1522
public class UnifiedQueryPlannerSqlV2Test extends UnifiedQueryTestBase {
1623

@@ -19,17 +26,35 @@ protected QueryType queryType() {
1926
return QueryType.SQL;
2027
}
2128

22-
@Test
23-
public void selectStar() {
24-
givenQuery("SELECT * FROM catalog.employees")
25-
.assertPlan(
26-
"""
27-
LogicalTableScan(table=[[catalog, employees]])
28-
""");
29+
@Before
30+
@Override
31+
public void setUp() {
32+
testSchema =
33+
new AbstractSchema() {
34+
@Override
35+
protected Map<String, Table> getTableMap() {
36+
return Map.of(
37+
"employees", createEmployeesTable(),
38+
"departments", createDepartmentsTable());
39+
}
40+
};
41+
42+
context = contextBuilder().build();
43+
planner = new UnifiedQueryPlanner(context);
44+
}
45+
46+
private Table createDepartmentsTable() {
47+
return SimpleTable.builder()
48+
.col("dept_id", INTEGER)
49+
.col("dept_name", VARCHAR)
50+
.row(new Object[] {1, "Engineering"})
51+
.row(new Object[] {2, "Sales"})
52+
.row(new Object[] {3, "Marketing"})
53+
.build();
2954
}
3055

3156
@Test
32-
public void selectStarFields() {
57+
public void selectStar() {
3358
givenQuery("SELECT * FROM catalog.employees")
3459
.assertPlan(
3560
"""
@@ -68,4 +93,53 @@ public void testFilterAndOrderBy() {
6893
LogicalTableScan(table=[[catalog, employees]])
6994
""");
7095
}
96+
97+
@Test
98+
public void testJoinTypes() {
99+
Map.of("JOIN", "inner", "LEFT JOIN", "left", "RIGHT JOIN", "right")
100+
.forEach(
101+
(syntax, type) ->
102+
givenQuery(
103+
"""
104+
SELECT * FROM catalog.employees %s catalog.departments
105+
ON employees.department = departments.dept_name
106+
"""
107+
.formatted(syntax))
108+
.assertPlan(
109+
"""
110+
LogicalJoin(condition=[=($3, $5)], joinType=[%s])
111+
LogicalTableScan(table=[[catalog, employees]])
112+
LogicalTableScan(table=[[catalog, departments]])
113+
"""
114+
.formatted(type)));
115+
}
116+
117+
@Test
118+
public void testCrossJoin() {
119+
givenQuery("SELECT * FROM catalog.employees CROSS JOIN catalog.departments")
120+
.assertPlan(
121+
"""
122+
LogicalJoin(condition=[true], joinType=[inner])
123+
LogicalTableScan(table=[[catalog, employees]])
124+
LogicalTableScan(table=[[catalog, departments]])
125+
""");
126+
}
127+
128+
@Test
129+
public void testJoinWithFilterAndOrderBy() {
130+
givenQuery(
131+
"""
132+
SELECT * FROM catalog.employees JOIN catalog.departments
133+
ON employees.department = departments.dept_name
134+
WHERE employees.age > 30 ORDER BY employees.name
135+
""")
136+
.assertPlan(
137+
"""
138+
LogicalSort(sort0=[$1], dir0=[ASC-nulls-first])
139+
LogicalFilter(condition=[>($2, 30)])
140+
LogicalJoin(condition=[=($3, $5)], joinType=[inner])
141+
LogicalTableScan(table=[[catalog, employees]])
142+
LogicalTableScan(table=[[catalog, departments]])
143+
""");
144+
}
71145
}

‎core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@
4848
import org.opensearch.sql.ast.expression.When;
4949
import org.opensearch.sql.ast.expression.WindowFunction;
5050
import org.opensearch.sql.ast.expression.Xor;
51-
import org.opensearch.sql.ast.expression.subquery.ExistsSubquery;
52-
import org.opensearch.sql.ast.expression.subquery.InSubquery;
5351
import org.opensearch.sql.ast.tree.Aggregation;
5452
import org.opensearch.sql.ast.tree.AppendPipe;
5553
import org.opensearch.sql.ast.tree.Bin;
@@ -773,12 +771,4 @@ public static UnresolvedPlan join(
773771
Optional.empty(),
774772
Argument.ArgumentMap.empty());
775773
}
776-
777-
public static InSubquery inSubquery(UnresolvedPlan query, UnresolvedExpression... values) {
778-
return new InSubquery(List.of(values), query);
779-
}
780-
781-
public static ExistsSubquery existsSubquery(UnresolvedPlan query) {
782-
return new ExistsSubquery(query);
783-
}
784774
}

‎integ-test/src/test/java/org/opensearch/sql/legacy/SqlLegacyEngineSanityIT.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,4 @@ public void testLeftJoinFallback() throws IOException {
4545
.formatted(TEST_INDEX_PEOPLE, TEST_INDEX_DOG));
4646
verifyDataRows(result, rows("Daenerys", "rex"));
4747
}
48-
49-
@Test
50-
public void testInSubqueryFallback() throws IOException {
51-
JSONObject result =
52-
executeQuery(
53-
"SELECT a.firstname FROM %s a WHERE a.firstname IN (SELECT holdersName FROM %s)"
54-
.formatted(TEST_INDEX_PEOPLE, TEST_INDEX_DOG));
55-
verifyDataRows(result, rows("Daenerys"), rows("Hattie"));
56-
}
5748
}

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -756,18 +756,4 @@ public UnresolvedPlan visitJoinClause(OpenSearchSQLParser.JoinClauseContext ctx)
756756
};
757757
assertNotNull(new SQLSyntaxParser().parse(query).accept(builder));
758758
}
759-
760-
@Test
761-
public void in_subquery_throws_syntax_check_exception() {
762-
assertThrows(
763-
SyntaxCheckException.class,
764-
() -> buildAST("SELECT * FROM t WHERE age IN (SELECT age FROM t2)"));
765-
}
766-
767-
@Test
768-
public void exists_subquery_throws_syntax_check_exception() {
769-
assertThrows(
770-
SyntaxCheckException.class,
771-
() -> buildAST("SELECT * FROM t WHERE EXISTS (SELECT 1 FROM t2)"));
772-
}
773759
}

0 commit comments

Comments
 (0)