From 485860059094c4c25b6bd067fe125242bd1f4a35 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 12 May 2026 16:23:15 -0700 Subject: [PATCH] Route unified SQL path through V2 ANTLR parser Switch SQL from Calcite's standard parser to V2 ANTLR parser + CalciteRelNodeVisitor (CustomVisitorStrategy). Add SqlV2QueryParser implementing UnifiedQueryParser. Register DatetimeExtension in UnifiedSqlSpec for UDT normalization. Comment out CoreExtension and SearchExtension (unused in V2 path). Extract QueryPlanAssertion mixin interface with fluent assertReturnType API for plan verification. Add V2 test classes: - UnifiedQueryPlannerSqlV2Test (basic queries) - UnifiedSqlV2SpecTest (V2 extensions over ANSI SQL) - UnifiedRelevanceSearchSqlV2Test (relevance functions) - DatetimeExtensionSqlTest (datetime UDT handling) Signed-off-by: Chen Dai --- api/build.gradle | 9 +- .../sql/api/UnifiedQueryContext.java | 4 +- .../sql/api/UnifiedQueryPlanner.java | 6 +- .../sql/api/parser/SqlV2QueryParser.java | 36 ++++ .../sql/api/spec/UnifiedSqlSpec.java | 7 +- .../sql/api/UnifiedFunctionSpecTest.java | 2 + .../sql/api/UnifiedQueryPlannerSqlTest.java | 2 + .../sql/api/UnifiedQueryPlannerSqlV2Test.java | 71 ++++++++ .../api/UnifiedRelevanceSearchSqlTest.java | 2 + .../api/UnifiedRelevanceSearchSqlV2Test.java | 135 +++++++++++++++ .../sql/api/UnifiedSqlSpecTest.java | 2 + .../sql/api/UnifiedSqlV2SpecTest.java | 102 ++++++++++++ .../datetime/DatetimeExtensionSqlTest.java | 93 +++++++++++ .../spec/datetime/DatetimeExtensionTest.java | 154 ++++++------------ .../sql/api/QueryPlanAssertion.java | 122 ++++++++++++++ .../sql/api/UnifiedQueryTestBase.java | 64 +------- 16 files changed, 633 insertions(+), 178 deletions(-) create mode 100644 api/src/main/java/org/opensearch/sql/api/parser/SqlV2QueryParser.java create mode 100644 api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java create mode 100644 api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlV2Test.java create mode 100644 api/src/test/java/org/opensearch/sql/api/UnifiedSqlV2SpecTest.java create mode 100644 api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionSqlTest.java create mode 100644 api/src/testFixtures/java/org/opensearch/sql/api/QueryPlanAssertion.java diff --git a/api/build.gradle b/api/build.gradle index 570efc6bb0e..638aa8700ce 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -13,6 +13,7 @@ plugins { dependencies { api project(':ppl') + api project(':sql') api group: 'org.apache.calcite', name: 'calcite-babel', version: '1.41.0' testImplementation testFixtures(project(':api')) @@ -69,13 +70,17 @@ jacocoTestCoverageVerification { limit { minimum = 0.9 } - } } afterEvaluate { classDirectories.setFrom(files(classDirectories.files.collect { fileTree(dir: it, - exclude: ['**/antlr/parser/**']) + // Calcite native SQL parser path replaced by SQL V2 ANTLR parser for now + exclude: ['**/antlr/parser/**', + '**/CalciteSqlQueryParser.class', + '**/UnifiedQueryPlanner$CalciteNativeStrategy.class', + '**/LateBindingFunctionRule.class', + '**/LateBindingFunctionRule$*.class']) })) } } diff --git a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java index 4372d5e05ba..7a885edd65a 100644 --- a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java +++ b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java @@ -27,8 +27,8 @@ import org.apache.calcite.tools.FrameworkConfig; import org.apache.calcite.tools.Frameworks; import org.apache.calcite.tools.Programs; -import org.opensearch.sql.api.parser.CalciteSqlQueryParser; import org.opensearch.sql.api.parser.PPLQueryParser; +import org.opensearch.sql.api.parser.SqlV2QueryParser; import org.opensearch.sql.api.parser.UnifiedQueryParser; import org.opensearch.sql.api.spec.LanguageSpec; import org.opensearch.sql.api.spec.UnifiedPplSpec; @@ -246,7 +246,7 @@ public UnifiedQueryContext build() { private UnifiedQueryParser createParser(CalcitePlanContext planContext, Settings settings) { return switch (queryType) { case PPL -> new PPLQueryParser(settings); - case SQL -> new CalciteSqlQueryParser(planContext); + case SQL -> new SqlV2QueryParser(); }; } diff --git a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java index 54a429e4cfb..50121d850f0 100644 --- a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java +++ b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java @@ -23,7 +23,6 @@ import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.calcite.CalciteRelNodeVisitor; import org.opensearch.sql.common.antlr.SyntaxCheckException; -import org.opensearch.sql.executor.QueryType; /** * {@code UnifiedQueryPlanner} provides a high-level API for parsing and analyzing queries using the @@ -45,10 +44,7 @@ public class UnifiedQueryPlanner { */ public UnifiedQueryPlanner(UnifiedQueryContext context) { this.context = context; - this.strategy = - context.getPlanContext().queryType == QueryType.SQL - ? new CalciteNativeStrategy(context) - : new CustomVisitorStrategy(context); + this.strategy = new CustomVisitorStrategy(context); } /** diff --git a/api/src/main/java/org/opensearch/sql/api/parser/SqlV2QueryParser.java b/api/src/main/java/org/opensearch/sql/api/parser/SqlV2QueryParser.java new file mode 100644 index 00000000000..827278a2119 --- /dev/null +++ b/api/src/main/java/org/opensearch/sql/api/parser/SqlV2QueryParser.java @@ -0,0 +1,36 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.parser; + +import org.antlr.v4.runtime.tree.ParseTree; +import org.opensearch.sql.ast.statement.Query; +import org.opensearch.sql.ast.statement.Statement; +import org.opensearch.sql.ast.tree.UnresolvedPlan; +import org.opensearch.sql.sql.antlr.SQLSyntaxParser; +import org.opensearch.sql.sql.parser.AstBuilder; +import org.opensearch.sql.sql.parser.AstStatementBuilder; + +/** SQL query parser that produces {@link UnresolvedPlan} using the V2 ANTLR grammar. */ +public class SqlV2QueryParser implements UnifiedQueryParser { + + /** Reusable ANTLR-based SQL syntax parser. Stateless and thread-safe. */ + private final SQLSyntaxParser syntaxParser = new SQLSyntaxParser(); + + @Override + public UnresolvedPlan parse(String query) { + ParseTree cst = syntaxParser.parse(query); + AstStatementBuilder astStmtBuilder = + new AstStatementBuilder( + new AstBuilder(query), AstStatementBuilder.StatementBuilderContext.builder().build()); + Statement statement = cst.accept(astStmtBuilder); + + if (statement instanceof Query) { + return ((Query) statement).getPlan(); + } + throw new UnsupportedOperationException( + "Only query statements are supported but got " + statement.getClass().getSimpleName()); + } +} diff --git a/api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java b/api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java index 28eeaa89abf..90382b5fafe 100644 --- a/api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java +++ b/api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java @@ -16,8 +16,7 @@ import org.apache.calcite.sql.parser.babel.SqlBabelParserImpl; import org.apache.calcite.sql.validate.SqlConformanceEnum; import org.apache.calcite.sql.validate.SqlValidator; -import org.opensearch.sql.api.spec.core.CoreExtension; -import org.opensearch.sql.api.spec.search.SearchExtension; +import org.opensearch.sql.api.spec.datetime.DatetimeExtension; /** * SQL language specification. Configures Calcite's parser, validator, and composable extensions for @@ -51,7 +50,9 @@ public static UnifiedSqlSpec extended() { Lex.BIG_QUERY, SqlBabelParserImpl.FACTORY, SqlConformanceEnum.BABEL, - List.of(new CoreExtension(), new SearchExtension())); + // CoreExtension and SearchExtension were for Calcite standard SQL pipeline and are + // unused in the V2 ANTLR parser path (CalciteRelNodeVisitor resolves functions directly) + List.of(/* new CoreExtension(), new SearchExtension(), */ new DatetimeExtension())); } @Override diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedFunctionSpecTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedFunctionSpecTest.java index a16fa116b42..0dfb3161627 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedFunctionSpecTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedFunctionSpecTest.java @@ -13,6 +13,7 @@ import java.sql.Timestamp; import org.apache.calcite.rel.RelNode; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.api.compiler.UnifiedQueryCompiler; import org.opensearch.sql.api.spec.UnifiedFunctionSpec; @@ -22,6 +23,7 @@ * Tests for scalar functions registered in {@link UnifiedFunctionSpec#SCALAR}. Verifies planning * (function resolves correctly) and execution (produces correct results in-memory). */ +@Ignore("Replaced by V2 ANTLR parser path for now") public class UnifiedFunctionSpecTest extends UnifiedQueryTestBase { private UnifiedQueryCompiler compiler; diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java index 855d3d2788d..4ce4059a83b 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java @@ -12,9 +12,11 @@ import java.util.Map; import org.apache.calcite.schema.Schema; import org.apache.calcite.schema.impl.AbstractSchema; +import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.executor.QueryType; +@Ignore("Replaced by V2 ANTLR parser path for now") public class UnifiedQueryPlannerSqlTest extends UnifiedQueryTestBase { private final AbstractSchema testDeepSchema = diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java new file mode 100644 index 00000000000..b9913bf450b --- /dev/null +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java @@ -0,0 +1,71 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api; + +import org.junit.Test; +import org.opensearch.sql.executor.QueryType; + +/** + * Tests for basic SQL query planning through the V2 ANTLR parser path. Covers SELECT, WHERE, ORDER + * BY operations that produce valid RelNode plans. + */ +public class UnifiedQueryPlannerSqlV2Test extends UnifiedQueryTestBase { + + @Override + protected QueryType queryType() { + return QueryType.SQL; + } + + @Test + public void selectStar() { + givenQuery("SELECT * FROM catalog.employees") + .assertPlan( + """ + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void selectStarFields() { + givenQuery("SELECT * FROM catalog.employees") + .assertPlan( + """ + LogicalTableScan(table=[[catalog, employees]]) + """) + .assertFields("id", "name", "age", "department"); + } + + @Test + public void testFilter() { + givenQuery("SELECT * FROM catalog.employees WHERE age > 30") + .assertPlan( + """ + LogicalFilter(condition=[>($2, 30)]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testOrderBy() { + givenQuery("SELECT * FROM catalog.employees ORDER BY age") + .assertPlan( + """ + LogicalSort(sort0=[$2], dir0=[ASC-nulls-first]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testFilterAndOrderBy() { + givenQuery("SELECT * FROM catalog.employees WHERE name = 'Alice' ORDER BY age") + .assertPlan( + """ + LogicalSort(sort0=[$2], dir0=[ASC-nulls-first]) + LogicalFilter(condition=[=($1, 'Alice')]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } +} diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java index 66df9c2e075..aa70f55bbb0 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java @@ -5,6 +5,7 @@ package org.opensearch.sql.api; +import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.executor.QueryType; @@ -13,6 +14,7 @@ * tests in {@link UnifiedRelevanceSearchTest} with equivalent SQL queries. Both paths produce * identical MAP-based plans for pushdown rules. */ +@Ignore("Replaced by V2 ANTLR parser path for now") public class UnifiedRelevanceSearchSqlTest extends UnifiedQueryTestBase { @Override diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlV2Test.java b/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlV2Test.java new file mode 100644 index 00000000000..4112f475003 --- /dev/null +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlV2Test.java @@ -0,0 +1,135 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api; + +import org.junit.Test; +import org.opensearch.sql.executor.QueryType; + +/** + * Tests for relevance search functions through the V2 ANTLR parser path. Covers match, + * match_phrase, multi_match, match_bool_prefix, match_phrase_prefix, simple_query_string, and + * query_string with bracket syntax. + */ +public class UnifiedRelevanceSearchSqlV2Test extends UnifiedQueryTestBase { + + @Override + protected QueryType queryType() { + return QueryType.SQL; + } + + @Test + public void match() { + givenQuery("SELECT * FROM catalog.employees WHERE match(name, 'John')") + .assertPlan( + """ + LogicalFilter(condition=[match(MAP('field', $1), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void matchWithOptions() { + givenQuery( + "SELECT * FROM catalog.employees WHERE match(name, 'John', operator='AND', boost=2.0)") + .assertPlan( + """ + LogicalFilter(condition=[match(MAP('field', $1), MAP('query', 'John':VARCHAR), MAP('operator', 'AND':VARCHAR), MAP('boost', '2.0':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void matchPhrase() { + givenQuery("SELECT * FROM catalog.employees WHERE match_phrase(name, 'John Doe')") + .assertPlan( + """ + LogicalFilter(condition=[match_phrase(MAP('field', $1), MAP('query', 'John Doe':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void matchBoolPrefix() { + givenQuery("SELECT * FROM catalog.employees WHERE match_bool_prefix(name, 'John')") + .assertPlan( + """ + LogicalFilter(condition=[match_bool_prefix(MAP('field', $1), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void matchPhrasePrefix() { + givenQuery("SELECT * FROM catalog.employees WHERE match_phrase_prefix(name, 'John')") + .assertPlan( + """ + LogicalFilter(condition=[match_phrase_prefix(MAP('field', $1), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void multiMatchBracketSyntax() { + givenQuery("SELECT * FROM catalog.employees WHERE multi_match(['name', 'department'], 'John')") + .assertPlan( + """ + LogicalFilter(condition=[multi_match(MAP('fields', MAP('name':VARCHAR, 1.0E0:DOUBLE, 'department':VARCHAR, 1.0E0:DOUBLE)), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void multiMatchWithFieldBoost() { + givenQuery( + """ + SELECT * FROM catalog.employees\ + WHERE multi_match(['name' ^ 2.0, 'department'], 'John')\ + """) + .assertPlan( + """ + LogicalFilter(condition=[multi_match(MAP('fields', MAP('name':VARCHAR, 2.0E0:DOUBLE, 'department':VARCHAR, 1.0E0:DOUBLE)), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void simpleQueryStringBracketSyntax() { + givenQuery( + """ + SELECT * FROM catalog.employees\ + WHERE simple_query_string(['name', 'department'], 'John')\ + """) + .assertPlan( + """ + LogicalFilter(condition=[simple_query_string(MAP('fields', MAP('name':VARCHAR, 1.0E0:DOUBLE, 'department':VARCHAR, 1.0E0:DOUBLE)), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void queryStringBracketSyntax() { + givenQuery( + """ + SELECT * FROM catalog.employees\ + WHERE query_string(['name', 'department'], 'John')\ + """) + .assertPlan( + """ + LogicalFilter(condition=[query_string(MAP('fields', MAP('name':VARCHAR, 1.0E0:DOUBLE, 'department':VARCHAR, 1.0E0:DOUBLE)), MAP('query', 'John':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void matchCombinedWithBooleanFilter() { + givenQuery("SELECT * FROM catalog.employees WHERE match(name, 'John') AND age > 25") + .assertPlan( + """ + LogicalFilter(condition=[AND(match(MAP('field', $1), MAP('query', 'John':VARCHAR)), >($2, 25))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } +} diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java index 97ddd07d0ac..27617f8a201 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java @@ -8,9 +8,11 @@ import java.util.Map; import org.apache.calcite.schema.Table; import org.apache.calcite.schema.impl.AbstractSchema; +import org.junit.Ignore; import org.junit.Test; import org.opensearch.sql.executor.QueryType; +@Ignore("Replaced by V2 ANTLR parser path for now") public class UnifiedSqlSpecTest extends UnifiedQueryTestBase { @Override diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedSqlV2SpecTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlV2SpecTest.java new file mode 100644 index 00000000000..a9dd137a61f --- /dev/null +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlV2SpecTest.java @@ -0,0 +1,102 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api; + +import java.util.Map; +import org.apache.calcite.schema.Table; +import org.apache.calcite.schema.impl.AbstractSchema; +import org.junit.Test; +import org.opensearch.sql.executor.QueryType; + +/** + * Tests for V2 ANTLR parser features that extend Calcite ANSI SQL. Focuses on differences + * documented in the SQL compatibility spec's "Differences from Standard SQL" section. + */ +public class UnifiedSqlV2SpecTest extends UnifiedQueryTestBase { + + @Override + protected QueryType queryType() { + return QueryType.SQL; + } + + @Override + protected UnifiedQueryContext.Builder contextBuilder() { + AbstractSchema schema = + new AbstractSchema() { + @Override + protected Map getTableMap() { + return Map.of( + "employees", createEmployeesTable(), + "logs-2024-01-01", createEmployeesTable()); + } + }; + return UnifiedQueryContext.builder().language(queryType()).catalog("catalog", schema); + } + + @Test + public void backtickQuotedTableName() { + // SQL V2 extends Calcite ANSI SQL by supporting MySQL-style backtick-quoted identifiers + givenQuery("SELECT * FROM catalog.`employees`") + .assertPlan( + """ + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void hyphenatedIndexName() { + // SQL V2 extends Calcite ANSI SQL by supporting hyphenated index names via backtick quoting + givenQuery("SELECT * FROM catalog.`logs-2024-01-01`") + .assertPlan( + """ + LogicalTableScan(table=[[catalog, logs-2024-01-01]]) + """); + } + + @Test + public void matchFunctionNotReservedWord() { + // SQL V2 extends Calcite ANSI SQL by de-reserving MATCH for use as a function name + givenQuery("SELECT * FROM catalog.employees WHERE match(name, 'Hattie')") + .assertPlan( + """ + LogicalFilter(condition=[match(MAP('field', $1), MAP('query', 'Hattie':VARCHAR))]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void stringToNumberCoercion() { + // SQL V2 extends Calcite ANSI SQL by lenient string-to-number coercion (MySQL-compatible) + givenQuery("SELECT * FROM catalog.employees WHERE age > '30'") + .assertPlan( + """ + LogicalFilter(condition=[>(SAFE_CAST($2), 30.0E0)]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void backslashEscapesInStrings() { + // SQL V2 extends Calcite ANSI SQL by supporting backslash escapes in single-quoted strings + givenQuery("SELECT * FROM catalog.employees WHERE name = 'it\\'s me'") + .assertPlan( + """ + LogicalFilter(condition=[=($1, 'it''s me')]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void doubleQuotedStringLiteral() { + // SQL V2 extends Calcite ANSI SQL by treating double-quoted values as string literals + givenQuery("SELECT * FROM catalog.employees WHERE name = \"hello\"") + .assertPlan( + """ + LogicalFilter(condition=[=($1, 'hello')]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } +} diff --git a/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionSqlTest.java b/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionSqlTest.java new file mode 100644 index 00000000000..3cae8abd9b5 --- /dev/null +++ b/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionSqlTest.java @@ -0,0 +1,93 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.spec.datetime; + +import static org.apache.calcite.sql.type.SqlTypeName.DATE; +import static org.apache.calcite.sql.type.SqlTypeName.INTEGER; +import static org.apache.calcite.sql.type.SqlTypeName.TIME; +import static org.apache.calcite.sql.type.SqlTypeName.TIMESTAMP; +import static org.apache.calcite.sql.type.SqlTypeName.VARCHAR; + +import java.util.Map; +import org.apache.calcite.schema.Table; +import org.apache.calcite.schema.impl.AbstractSchema; +import org.junit.Test; +import org.opensearch.sql.api.UnifiedQueryContext; +import org.opensearch.sql.api.UnifiedQueryTestBase; +import org.opensearch.sql.executor.QueryType; + +/** + * Tests that DatetimeExtension post-analysis rules (UDT normalization and output VARCHAR cast) + * apply correctly to the SQL V2 parser path through CalciteRelNodeVisitor. + */ +public class DatetimeExtensionSqlTest extends UnifiedQueryTestBase { + + @Override + protected QueryType queryType() { + return QueryType.SQL; + } + + @Override + protected UnifiedQueryContext.Builder contextBuilder() { + return UnifiedQueryContext.builder() + .language(QueryType.SQL) + .catalog( + DEFAULT_CATALOG, + new AbstractSchema() { + @Override + protected Map getTableMap() { + return Map.of("events", createEventsTable()); + } + }); + } + + private Table createEventsTable() { + return SimpleTable.builder() + .col("id", INTEGER) + .col("name", VARCHAR) + .col("hire_date", DATE) + .col("start_time", TIME) + .col("created_at", TIMESTAMP) + .row(new Object[] {1, "Alice", 19738, 43200000, 1705305600000L}) + .row(new Object[] {2, "Bob", 19894, 50400000, 1718841600000L}) + .build(); + } + + @Test + public void testAllStandardDatetimeTypesCastToVarchar() { + givenQuery("SELECT * FROM catalog.events") + .assertPlan( + """ + LogicalProject(id=[$0], name=[$1], hire_date=[CAST($2):VARCHAR NOT NULL], start_time=[CAST($3):VARCHAR NOT NULL], created_at=[CAST($4):VARCHAR NOT NULL]) + LogicalTableScan(table=[[catalog, events]]) + """); + } + + @Test + public void testFilterWithTimestampLiteral() { + givenQuery("SELECT * FROM catalog.events WHERE created_at > '2024-01-01T00:00:00Z'") + .assertPlan( + """ + LogicalProject(id=[$0], name=[$1], hire_date=[CAST($2):VARCHAR NOT NULL], start_time=[CAST($3):VARCHAR NOT NULL], created_at=[CAST($4):VARCHAR NOT NULL]) + LogicalFilter(condition=[>($4, TIMESTAMP('2024-01-01T00:00:00Z':VARCHAR))]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("TIMESTAMP", TIMESTAMP, 9); + } + + @Test + public void testComparisonWithDatetimeUdf() { + givenQuery("SELECT * FROM catalog.events WHERE created_at < DATE(name)") + .assertPlan( + """ + LogicalProject(id=[$0], name=[$1], hire_date=[CAST($2):VARCHAR NOT NULL], start_time=[CAST($3):VARCHAR NOT NULL], created_at=[CAST($4):VARCHAR NOT NULL]) + LogicalFilter(condition=[<($4, TIMESTAMP(DATE($1)))]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("DATE", DATE) + .assertReturnType("TIMESTAMP", TIMESTAMP, 9); + } +} diff --git a/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java b/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java index fc089150109..c7c1b776e72 100644 --- a/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java +++ b/api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java @@ -11,21 +11,13 @@ import static org.apache.calcite.sql.type.SqlTypeName.TIME; import static org.apache.calcite.sql.type.SqlTypeName.TIMESTAMP; import static org.apache.calcite.sql.type.SqlTypeName.VARCHAR; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; -import org.apache.calcite.rel.RelHomogeneousShuttle; import org.apache.calcite.rel.RelNode; -import org.apache.calcite.rex.RexCall; -import org.apache.calcite.rex.RexNode; -import org.apache.calcite.rex.RexShuttle; import org.apache.calcite.schema.Table; import org.apache.calcite.schema.impl.AbstractSchema; -import org.apache.calcite.sql.type.SqlTypeName; import org.junit.Before; import org.junit.Test; import org.opensearch.sql.api.ResultSetAssertion; @@ -72,83 +64,73 @@ private Table createEventsTable() { @Test public void testUdfResultNormalizedAndCastToVarchar() { - var plan = - givenQuery( - """ - source = catalog.events \ - | eval d = DATE(name), t = TIME(name), ts = TIMESTAMP(name) \ - | fields d, t, ts\ - """) - .assertPlan( - """ - LogicalProject(d=[CAST($0):VARCHAR], t=[CAST($1):VARCHAR], ts=[CAST($2):VARCHAR]) - LogicalProject(d=[DATE($1)], t=[TIME($1)], ts=[TIMESTAMP($1)]) - LogicalTableScan(table=[[catalog, events]]) - """) - .plan(); - assertCallType(plan, "DATE", DATE); - assertCallType(plan, "TIME", TIME, 9); - assertCallType(plan, "TIMESTAMP", TIMESTAMP, 9); + givenQuery( + """ + source = catalog.events \ + | eval d = DATE(name), t = TIME(name), ts = TIMESTAMP(name) \ + | fields d, t, ts\ + """) + .assertPlan( + """ + LogicalProject(d=[CAST($0):VARCHAR], t=[CAST($1):VARCHAR], ts=[CAST($2):VARCHAR]) + LogicalProject(d=[DATE($1)], t=[TIME($1)], ts=[TIMESTAMP($1)]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("DATE", DATE) + .assertReturnType("TIME", TIME, 9) + .assertReturnType("TIMESTAMP", TIMESTAMP, 9); } @Test public void testNestedUdfCallsNormalized() { - var plan = - givenQuery("source = catalog.events | eval d = DATEDIFF(DATE(name), DATE(name)) | fields d") - .assertPlan( - """ - LogicalProject(d=[DATEDIFF(DATE($1), DATE($1))]) - LogicalTableScan(table=[[catalog, events]]) - """) - .plan(); - assertCallType(plan, "DATE", DATE); - assertCallType(plan, "DATEDIFF", BIGINT); + givenQuery("source = catalog.events | eval d = DATEDIFF(DATE(name), DATE(name)) | fields d") + .assertPlan( + """ + LogicalProject(d=[DATEDIFF(DATE($1), DATE($1))]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("DATE", DATE) + .assertReturnType("DATEDIFF", BIGINT); } @Test public void testDateLiteralCastToVarchar() { - var plan = - givenQuery("source = catalog.events | eval d = DATE('2024-01-01') | fields d") - .assertPlan( - """ - LogicalProject(d=[CAST($0):VARCHAR]) - LogicalProject(d=[DATE('2024-01-01':VARCHAR)]) - LogicalTableScan(table=[[catalog, events]]) - """) - .plan(); - assertCallType(plan, "DATE", DATE); + givenQuery("source = catalog.events | eval d = DATE('2024-01-01') | fields d") + .assertPlan( + """ + LogicalProject(d=[CAST($0):VARCHAR]) + LogicalProject(d=[DATE('2024-01-01':VARCHAR)]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("DATE", DATE); } @Test public void testFilterWithTimestampLiteral() { - var plan = - givenQuery( - """ - source = catalog.events | where created_at > "2024-01-01T00:00:00Z" | fields id\ - """) - .assertPlan( - """ - LogicalProject(id=[$0]) - LogicalFilter(condition=[>($4, TIMESTAMP('2024-01-01T00:00:00Z':VARCHAR))]) - LogicalTableScan(table=[[catalog, events]]) - """) - .plan(); - assertCallType(plan, "TIMESTAMP", TIMESTAMP, 9); + givenQuery( + """ + source = catalog.events | where created_at > "2024-01-01T00:00:00Z" | fields id\ + """) + .assertPlan( + """ + LogicalProject(id=[$0]) + LogicalFilter(condition=[>($4, TIMESTAMP('2024-01-01T00:00:00Z':VARCHAR))]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("TIMESTAMP", TIMESTAMP, 9); } @Test public void testComparisonWithDatetimeUdf() { - var plan = - givenQuery("source = catalog.events | where created_at < DATE(name) | fields id") - .assertPlan( - """ - LogicalProject(id=[$0]) - LogicalFilter(condition=[<($4, TIMESTAMP(DATE($1)))]) - LogicalTableScan(table=[[catalog, events]]) - """) - .plan(); - assertCallType(plan, "DATE", DATE); - assertCallType(plan, "TIMESTAMP", TIMESTAMP, 9); + givenQuery("source = catalog.events | where created_at < DATE(name) | fields id") + .assertPlan( + """ + LogicalProject(id=[$0]) + LogicalFilter(condition=[<($4, TIMESTAMP(DATE($1)))]) + LogicalTableScan(table=[[catalog, events]]) + """) + .assertReturnType("DATE", DATE) + .assertReturnType("TIMESTAMP", TIMESTAMP, 9); } @Test @@ -188,38 +170,4 @@ public void testOutputCastCanCompileAndExecute() throws Exception { row("2024-06-20", "14:00:00", "2024-06-20 00:00:00")); } } - - private static void assertCallType(RelNode plan, String operatorName, SqlTypeName expectedType) { - assertCallType(plan, operatorName, expectedType, -1); - } - - private static void assertCallType( - RelNode plan, String operatorName, SqlTypeName expectedType, int expectedPrecision) { - AtomicReference ref = new AtomicReference<>(); - plan.accept( - new RelHomogeneousShuttle() { - @Override - public RelNode visit(RelNode other) { - RelNode visited = super.visit(other); - visited.accept( - new RexShuttle() { - @Override - public RexNode visitCall(RexCall call) { - if (ref.get() == null - && call.getOperator().getName().equalsIgnoreCase(operatorName)) { - ref.set(call); - } - return super.visitCall(call); - } - }); - return visited; - } - }); - assertNotNull("No RexCall found for: " + operatorName, ref.get()); - assertEquals(operatorName + " type", expectedType, ref.get().getType().getSqlTypeName()); - if (expectedPrecision >= 0) { - assertEquals( - operatorName + " precision", expectedPrecision, ref.get().getType().getPrecision()); - } - } } diff --git a/api/src/testFixtures/java/org/opensearch/sql/api/QueryPlanAssertion.java b/api/src/testFixtures/java/org/opensearch/sql/api/QueryPlanAssertion.java new file mode 100644 index 00000000000..0fd4f1bcc04 --- /dev/null +++ b/api/src/testFixtures/java/org/opensearch/sql/api/QueryPlanAssertion.java @@ -0,0 +1,122 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import lombok.RequiredArgsConstructor; +import org.apache.calcite.plan.RelOptUtil; +import org.apache.calcite.rel.RelHomogeneousShuttle; +import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rex.RexCall; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexShuttle; +import org.apache.calcite.sql.type.SqlTypeName; + +/** + * Mixin interface providing fluent assertion API for query plan verification. Tests can implement + * this interface to gain access to {@link QueryAssert} and {@link QueryErrorAssert} for asserting + * logical plan structure, field names, and operator return types. + */ +public interface QueryPlanAssertion { + + /** Fluent assertion on a query's logical plan. */ + @RequiredArgsConstructor + class QueryAssert { + private final RelNode plan; + + /** Assert the logical plan matches the expected tree string. */ + public QueryAssert assertPlan(String expected) { + assertEquals( + expected.stripTrailing(), + RelOptUtil.toString(plan).replaceAll("\\r\\n", "\n").stripTrailing()); + return this; + } + + /** Assert the logical plan contains the expected substring. */ + public QueryAssert assertPlanContains(String expected) { + String planStr = RelOptUtil.toString(plan).replaceAll("\\r\\n", "\n"); + assertTrue( + "Expected plan to contain: " + expected + "\nActual plan:\n" + planStr, + planStr.contains(expected)); + return this; + } + + /** Assert the output field names match. */ + public QueryAssert assertFields(String... names) { + assertEquals(List.of(names), plan.getRowType().getFieldNames()); + return this; + } + + /** Assert a function/operator in the plan has the expected return type. */ + public QueryAssert assertReturnType(String operatorName, SqlTypeName expectedType) { + return assertReturnType(operatorName, expectedType, -1); + } + + /** Assert a function/operator in the plan has the expected return type and precision. */ + public QueryAssert assertReturnType( + String operatorName, SqlTypeName expectedType, int expectedPrecision) { + RexCall call = findCall(operatorName); + assertNotNull("No RexCall found for: " + operatorName, call); + assertEquals(operatorName + " type", expectedType, call.getType().getSqlTypeName()); + if (expectedPrecision >= 0) { + assertEquals(operatorName + " precision", expectedPrecision, call.getType().getPrecision()); + } + return this; + } + + /** Access the underlying plan for custom assertions. */ + public RelNode plan() { + return plan; + } + + private RexCall findCall(String operatorName) { + AtomicReference ref = new AtomicReference<>(); + plan.accept( + new RelHomogeneousShuttle() { + @Override + public RelNode visit(RelNode other) { + RelNode visited = super.visit(other); + visited.accept( + new RexShuttle() { + @Override + public RexNode visitCall(RexCall call) { + if (ref.get() == null + && call.getOperator().getName().equalsIgnoreCase(operatorName)) { + ref.set(call); + } + return super.visitCall(call); + } + }); + return visited; + } + }); + return ref.get(); + } + } + + /** Fluent assertion on a query planning error. */ + @RequiredArgsConstructor + class QueryErrorAssert { + private final Exception error; + + /** Assert the root cause error message contains the expected substring. */ + public QueryErrorAssert assertErrorMessage(String expected) { + Throwable cause = error; + while (cause.getCause() != null) { + cause = cause.getCause(); + } + String msg = cause.getMessage() != null ? cause.getMessage() : cause.getClass().getName(); + assertTrue( + "Expected error to contain: " + expected + "\nActual: " + msg, msg.contains(expected)); + return this; + } + } +} diff --git a/api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.java b/api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.java index 42df6c5a7ee..b04cd68cf54 100644 --- a/api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.java +++ b/api/src/testFixtures/java/org/opensearch/sql/api/UnifiedQueryTestBase.java @@ -7,8 +7,6 @@ import static org.apache.calcite.sql.type.SqlTypeName.INTEGER; import static org.apache.calcite.sql.type.SqlTypeName.VARCHAR; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import java.util.List; import java.util.Map; @@ -17,8 +15,6 @@ import org.apache.calcite.DataContext; import org.apache.calcite.linq4j.Enumerable; import org.apache.calcite.linq4j.Linq4j; -import org.apache.calcite.plan.RelOptUtil; -import org.apache.calcite.rel.RelNode; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.schema.ScannableTable; @@ -35,7 +31,7 @@ import org.opensearch.sql.executor.QueryType; /** Base class for unified query tests providing common test schema and utilities. */ -public abstract class UnifiedQueryTestBase { +public abstract class UnifiedQueryTestBase implements QueryPlanAssertion { /** Default catalog name */ protected static final String DEFAULT_CATALOG = "catalog"; @@ -158,62 +154,4 @@ protected QueryErrorAssert givenInvalidQuery(String query) { return new QueryErrorAssert(e); } } - - /** Fluent assertion on a query planning error. */ - protected static class QueryErrorAssert { - private final Exception error; - - QueryErrorAssert(Exception error) { - this.error = error; - } - - /** Assert the root cause error message contains the expected substring. */ - public QueryErrorAssert assertErrorMessage(String expected) { - Throwable cause = error; - while (cause.getCause() != null) { - cause = cause.getCause(); - } - String msg = cause.getMessage() != null ? cause.getMessage() : cause.getClass().getName(); - assertTrue( - "Expected error to contain: " + expected + "\nActual: " + msg, msg.contains(expected)); - return this; - } - } - - /** Fluent assertion on a query's logical plan. */ - protected static class QueryAssert { - private final RelNode plan; - - QueryAssert(RelNode plan) { - this.plan = plan; - } - - /** Assert the logical plan matches the expected tree string. */ - public QueryAssert assertPlan(String expected) { - assertEquals( - expected.stripTrailing(), - RelOptUtil.toString(plan).replaceAll("\\r\\n", "\n").stripTrailing()); - return this; - } - - /** Assert the logical plan contains the expected substring. */ - public QueryAssert assertPlanContains(String expected) { - String planStr = RelOptUtil.toString(plan).replaceAll("\\r\\n", "\n"); - assertTrue( - "Expected plan to contain: " + expected + "\nActual plan:\n" + planStr, - planStr.contains(expected)); - return this; - } - - /** Assert the output field names match. */ - public QueryAssert assertFields(String... names) { - assertEquals(List.of(names), plan.getRowType().getFieldNames()); - return this; - } - - /** Access the underlying plan for custom assertions. */ - public RelNode plan() { - return plan; - } - } }