From f0d23628fa65f1ebdf1904faaaada8ed07ae245b Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 16 Apr 2026 10:14:38 -0700 Subject: [PATCH 1/3] feat(api): Define unified SQL language spec with composable extensions (#5346) Introduce UnifiedSqlSpec that configures Calcite's parser/validator seams to accept OpenSearch SQL syntax, with a composable SqlExtension interface for domain-specific functions and AST rewriters. - Lex.BIG_QUERY: hyphenated identifiers, backtick quoting - SqlBabelParserImpl: de-reserves MATCH and most keywords - SqlConformanceEnum.BABEL: lenient GROUP BY, LIMIT, optional FROM - SearchExtension: relevance functions + NamedArgRewriter Signed-off-by: Chen Dai --- api/build.gradle | 1 + .../sql/api/UnifiedQueryContext.java | 43 +++---- .../sql/api/UnifiedQueryPlanner.java | 9 +- .../opensearch/sql/api/spec/LanguageSpec.java | 89 ++++++++++++++ .../sql/api/spec/UnifiedSqlSpec.java | 68 +++++++++++ .../search}/NamedArgRewriter.java | 2 +- .../sql/api/spec/search/SearchExtension.java | 27 +++++ .../sql/api/UnifiedQueryPlannerSqlTest.java | 12 +- .../api/UnifiedRelevanceSearchSqlTest.java | 6 +- .../sql/api/UnifiedSqlSpecTest.java | 114 ++++++++++++++++++ .../search}/NamedArgRewriterTest.java | 2 +- 11 files changed, 341 insertions(+), 32 deletions(-) create mode 100644 api/src/main/java/org/opensearch/sql/api/spec/LanguageSpec.java create mode 100644 api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java rename api/src/main/java/org/opensearch/sql/api/{parser => spec/search}/NamedArgRewriter.java (98%) create mode 100644 api/src/main/java/org/opensearch/sql/api/spec/search/SearchExtension.java create mode 100644 api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java rename api/src/test/java/org/opensearch/sql/api/{parser => spec/search}/NamedArgRewriterTest.java (98%) diff --git a/api/build.gradle b/api/build.gradle index fb4cafe79d8..570efc6bb0e 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -13,6 +13,7 @@ plugins { dependencies { api project(':ppl') + api group: 'org.apache.calcite', name: 'calcite-babel', version: '1.41.0' testImplementation testFixtures(project(':api')) testImplementation group: 'junit', name: 'junit', version: '4.13.2' 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 a8c4a302110..8f426f4e70e 100644 --- a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java +++ b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java @@ -17,22 +17,19 @@ import java.util.concurrent.Callable; import lombok.AllArgsConstructor; import lombok.Getter; -import org.apache.calcite.avatica.util.Casing; import org.apache.calcite.jdbc.CalciteSchema; import org.apache.calcite.plan.RelTraitDef; import org.apache.calcite.rel.metadata.DefaultRelMetadataProvider; import org.apache.calcite.schema.Schema; import org.apache.calcite.schema.SchemaPlus; -import org.apache.calcite.sql.fun.SqlStdOperatorTable; -import org.apache.calcite.sql.parser.SqlParser; -import org.apache.calcite.sql.util.SqlOperatorTables; 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.UnifiedQueryParser; -import org.opensearch.sql.api.spec.UnifiedFunctionSpec; +import org.opensearch.sql.api.spec.LanguageSpec; +import org.opensearch.sql.api.spec.UnifiedSqlSpec; import org.opensearch.sql.calcite.CalcitePlanContext; import org.opensearch.sql.calcite.SysLimit; import org.opensearch.sql.common.setting.Settings; @@ -60,6 +57,9 @@ public class UnifiedQueryContext implements AutoCloseable { /** Query parser created eagerly from this context's configuration. */ private final UnifiedQueryParser parser; + /** Language spec for SQL, or null for PPL (TODO: converge PPL onto LanguageSpec). */ + private final LanguageSpec langSpec; + /** * Returns the profiling result. Call after query execution to retrieve collected metrics. Returns * empty if profiling was not enabled. @@ -208,12 +208,14 @@ public Builder setting(String name, Object value) { public UnifiedQueryContext build() { Objects.requireNonNull(queryType, "Must specify language before build"); + LanguageSpec langSpec = (queryType == QueryType.SQL) ? UnifiedSqlSpec.extended() : null; Settings settings = buildSettings(); CalcitePlanContext planContext = CalcitePlanContext.create( - buildFrameworkConfig(), SysLimit.fromSettings(settings), queryType); + buildFrameworkConfig(langSpec), SysLimit.fromSettings(settings), queryType); QueryProfiling.activate(profiling); - return new UnifiedQueryContext(planContext, settings, createParser(planContext, settings)); + return new UnifiedQueryContext( + planContext, settings, createParser(planContext, settings), langSpec); } private UnifiedQueryParser createParser(CalcitePlanContext planContext, Settings settings) { @@ -239,25 +241,24 @@ public List getSettings() { } @SuppressWarnings({"rawtypes"}) - private FrameworkConfig buildFrameworkConfig() { + private FrameworkConfig buildFrameworkConfig(LanguageSpec langSpec) { SchemaPlus rootSchema = CalciteSchema.createRootSchema(true, cacheMetadata).plus(); catalogs.forEach(rootSchema::add); SchemaPlus defaultSchema = findSchemaByPath(rootSchema, defaultNamespace); - return Frameworks.newConfigBuilder() - .parserConfig(buildParserConfig()) - .operatorTable( - SqlOperatorTables.chain( - SqlStdOperatorTable.instance(), UnifiedFunctionSpec.RELEVANCE.operatorTable())) - .defaultSchema(defaultSchema) - .traitDefs((List) null) - .programs(Programs.calc(DefaultRelMetadataProvider.INSTANCE)) - .build(); - } + Frameworks.ConfigBuilder builder = + Frameworks.newConfigBuilder() + .defaultSchema(defaultSchema) + .traitDefs((List) null) + .programs(Programs.calc(DefaultRelMetadataProvider.INSTANCE)); - private SqlParser.Config buildParserConfig() { - // Preserve identifier case for lowercase OpenSearch index names - return SqlParser.Config.DEFAULT.withUnquotedCasing(Casing.UNCHANGED); + if (langSpec != null) { + builder + .parserConfig(langSpec.parserConfig()) + .sqlValidatorConfig(langSpec.validatorConfig()) + .operatorTable(langSpec.operatorTable()); + } + return builder.build(); } private SchemaPlus findSchemaByPath(SchemaPlus rootSchema, String defaultPath) { 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 34872e2cd2e..edf9ae50e18 100644 --- a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java +++ b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryPlanner.java @@ -16,9 +16,9 @@ import org.apache.calcite.rel.logical.LogicalSort; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.util.SqlVisitor; import org.apache.calcite.tools.Frameworks; import org.apache.calcite.tools.Planner; -import org.opensearch.sql.api.parser.NamedArgRewriter; import org.opensearch.sql.api.parser.UnifiedQueryParser; import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.calcite.CalciteRelNodeVisitor; @@ -87,7 +87,12 @@ public RelNode plan(String query) throws Exception { "Only query statements are supported. Got: " + parsed.getKind()); } - SqlNode rewritten = parsed.accept(NamedArgRewriter.INSTANCE); + // TODO: move post-parse rewriting into CalciteSqlQueryParser + SqlNode rewritten = parsed; + for (SqlVisitor visitor : context.getLangSpec().postParseRules()) { + rewritten = rewritten.accept(visitor); + } + SqlNode validated = planner.validate(rewritten); RelRoot relRoot = planner.rel(validated); return relRoot.project(); diff --git a/api/src/main/java/org/opensearch/sql/api/spec/LanguageSpec.java b/api/src/main/java/org/opensearch/sql/api/spec/LanguageSpec.java new file mode 100644 index 00000000000..89167dc27a5 --- /dev/null +++ b/api/src/main/java/org/opensearch/sql/api/spec/LanguageSpec.java @@ -0,0 +1,89 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.spec; + +import java.util.ArrayList; +import java.util.List; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlOperatorTable; +import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.util.SqlOperatorTables; +import org.apache.calcite.sql.util.SqlVisitor; +import org.apache.calcite.sql.validate.SqlValidator; + +/** + * Language specification defining the dialect the engine accepts. Provides parser configuration, + * validator configuration, and composable {@link LanguageExtension}s that contribute operators and + * post-parse rewrite rules. + * + *

Implementations define a complete language surface — for example, {@link UnifiedSqlSpec} + * provides ANSI and extended SQL modes. A future PPL spec would implement this same interface once + * PPL converges on the Calcite pipeline. + */ +public interface LanguageSpec { + + /** + * A composable language extension that contributes operators and post-parse rewrite rules. All + * methods have defaults so extensions only override what they need. + */ + interface LanguageExtension { + + /** + * Operators (functions, aggregates) this extension adds. Chained with the standard operator + * table during validation. + */ + default SqlOperatorTable operators() { + return SqlOperatorTables.of(); + } + + /** + * AST rewrite rules applied after parsing and before validation. Each visitor transforms the + * parse tree (e.g., rewriting named arguments into MAP literals). + */ + default List> postParseRules() { + return List.of(); + } + } + + /** + * Parser configuration controlling how SQL text is tokenized and parsed into a parse tree, + * including parser factory, lexical rules, and conformance. + */ + SqlParser.Config parserConfig(); + + /** + * Validator configuration controlling what SQL semantics the validator accepts, such as GROUP BY + * behavior, LIMIT syntax, and type coercion. + */ + SqlValidator.Config validatorConfig(); + + /** + * Language extensions registered with this spec. Each extension contributes operators and + * post-parse rewrite rules that are composed by {@link #operatorTable()} and {@link + * #postParseRules()}. + */ + List extensions(); + + /** + * Chained operator table combining the standard Calcite operators with all operators contributed + * by registered extensions. + */ + default SqlOperatorTable operatorTable() { + List tables = new ArrayList<>(); + tables.add(SqlStdOperatorTable.instance()); + extensions().forEach(ext -> tables.add(ext.operators())); + return SqlOperatorTables.chain(tables); + } + + /** + * All post-parse rewrite rules from registered extensions, flattened in registration order. + * Applied to the parse tree after parsing and before validation. + */ + default List> postParseRules() { + return extensions().stream().flatMap(ext -> ext.postParseRules().stream()).toList(); + } +} 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 new file mode 100644 index 00000000000..a5433f015fa --- /dev/null +++ b/api/src/main/java/org/opensearch/sql/api/spec/UnifiedSqlSpec.java @@ -0,0 +1,68 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.spec; + +import java.util.List; +import lombok.AccessLevel; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.experimental.Accessors; +import org.apache.calcite.config.Lex; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.parser.SqlParserImplFactory; +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.search.SearchExtension; + +/** + * SQL language specification. Configures Calcite's parser, validator, and composable extensions for + * OpenSearch SQL compatibility. + * + *

Use {@link #extended()} for the default configuration with lenient syntax, hyphenated + * identifiers, and search functions. + */ +@RequiredArgsConstructor(access = AccessLevel.PRIVATE) +@Accessors(fluent = true) +public class UnifiedSqlSpec implements LanguageSpec { + + /** Lexical rules: identifier quoting, character escaping, and special identifier support. */ + private final Lex lex; + + /** Parser implementation: controls keyword reservation and grammar extensions. */ + private final SqlParserImplFactory parserFactory; + + /** Validation rules: what SQL semantics the validator accepts (GROUP BY, LIMIT, coercion). */ + private final SqlConformanceEnum conformance; + + /** Composable extensions contributing operators and post-parse rewrite rules. */ + @Getter private final List extensions; + + /** + * Extended SQL spec: Babel parser, BIG_QUERY lex (hyphenated identifiers, backtick quoting), + * BABEL conformance (lenient GROUP BY, LIMIT, optional FROM), and search functions. + */ + public static UnifiedSqlSpec extended() { + return new UnifiedSqlSpec( + Lex.BIG_QUERY, + SqlBabelParserImpl.FACTORY, + SqlConformanceEnum.BABEL, + List.of(new SearchExtension())); + } + + @Override + public SqlParser.Config parserConfig() { + return SqlParser.config() + .withParserFactory(parserFactory) + .withLex(lex) + .withConformance(conformance); + } + + @Override + public SqlValidator.Config validatorConfig() { + return SqlValidator.Config.DEFAULT.withConformance(conformance); + } +} diff --git a/api/src/main/java/org/opensearch/sql/api/parser/NamedArgRewriter.java b/api/src/main/java/org/opensearch/sql/api/spec/search/NamedArgRewriter.java similarity index 98% rename from api/src/main/java/org/opensearch/sql/api/parser/NamedArgRewriter.java rename to api/src/main/java/org/opensearch/sql/api/spec/search/NamedArgRewriter.java index 2a8e24b6e87..8627a76f2cf 100644 --- a/api/src/main/java/org/opensearch/sql/api/parser/NamedArgRewriter.java +++ b/api/src/main/java/org/opensearch/sql/api/spec/search/NamedArgRewriter.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package org.opensearch.sql.api.parser; +package org.opensearch.sql.api.spec.search; import java.util.List; import lombok.AccessLevel; diff --git a/api/src/main/java/org/opensearch/sql/api/spec/search/SearchExtension.java b/api/src/main/java/org/opensearch/sql/api/spec/search/SearchExtension.java new file mode 100644 index 00000000000..159560067c5 --- /dev/null +++ b/api/src/main/java/org/opensearch/sql/api/spec/search/SearchExtension.java @@ -0,0 +1,27 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.spec.search; + +import java.util.List; +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlOperatorTable; +import org.apache.calcite.sql.util.SqlVisitor; +import org.opensearch.sql.api.spec.LanguageSpec; +import org.opensearch.sql.api.spec.UnifiedFunctionSpec; + +/** Search Extension: relevance functions and named argument rewriting. */ +public class SearchExtension implements LanguageSpec.LanguageExtension { + + @Override + public SqlOperatorTable operators() { + return UnifiedFunctionSpec.RELEVANCE.operatorTable(); + } + + @Override + public List> postParseRules() { + return List.of(NamedArgRewriter.INSTANCE); + } +} 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 70079cc9272..0383ac4ec2d 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java @@ -245,17 +245,21 @@ public void testNonQueryStatementsBlockedByWhitelist() { @Test public void testNonQueryStatementsBlockedByParser() { - List.of( + // Babel parser rejects CREATE MATERIALIZED VIEW + givenInvalidQuery( """ CREATE MATERIALIZED VIEW mv AS SELECT department, count(*) FROM catalog.employees GROUP BY department\ - """, + """) + .assertErrorMessage("Encountered"); + + // Babel parser accepts SHOW TABLES but it's blocked by query-type whitelist + givenInvalidQuery( """ SHOW TABLES\ """) - .forEach( - sql -> givenInvalidQuery(sql).assertErrorMessage("Incorrect syntax near the keyword")); + .assertErrorMessage("Only query statements are supported"); } } 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 37490141cf1..66df9c2e075 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedRelevanceSearchSqlTest.java @@ -25,7 +25,7 @@ public void testMatch() { givenQuery( """ SELECT * FROM catalog.employees - WHERE "match"(name, 'John')\ + WHERE match(name, 'John')\ """) .assertPlan( """ @@ -100,7 +100,7 @@ public void testMatchWithOptions() { givenQuery( """ SELECT * FROM catalog.employees - WHERE "match"(name, 'John', operator='AND', boost=2.0)\ + WHERE match(name, 'John', operator='AND', boost=2.0)\ """) .assertPlanContains( "match(MAP('field', $1), MAP('query', 'John')," @@ -112,7 +112,7 @@ public void testMatchMissingArguments() { givenInvalidQuery( """ SELECT * FROM catalog.employees - WHERE "match"('John')\ + WHERE match('John')\ """) .assertErrorMessage( "No match found for function signature match(<(CHAR(5), CHAR(4)) MAP>)"); diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java new file mode 100644 index 00000000000..15868ac7530 --- /dev/null +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java @@ -0,0 +1,114 @@ +/* + * 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; + +public class UnifiedSqlSpecTest 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", createEmployeesTable()); + } + }; + return UnifiedQueryContext.builder() + .language(queryType()) + .catalog(DEFAULT_CATALOG, schema) + .defaultNamespace(DEFAULT_CATALOG); + } + + @Test + public void hyphenatedTableIdentifier() { + givenQuery("SELECT * FROM logs-2024-01") + .assertPlanContains("LogicalTableScan(table=[[catalog, logs-2024-01]])"); + } + + @Test + public void backtickQuotedIdentifiers() { + givenQuery("SELECT `name` FROM employees").assertPlanContains("LogicalProject(name=[$1])"); + } + + @Test + public void doubleQuotedStringLiteral() { + givenQuery("SELECT \"Hello\" FROM employees").assertPlanContains("LogicalProject"); + } + + @Test + public void matchNotReserved() { + givenQuery("SELECT * FROM employees WHERE match(name, 'Hattie')") + .assertPlanContains("LogicalFilter"); + } + + @Test + public void reservedWordAsAlias() { + givenQuery("SELECT age AS year FROM employees").assertPlanContains("LogicalProject(year=[$2])"); + } + + @Test + public void limitSyntax() { + givenQuery("SELECT * FROM employees LIMIT 10").assertPlanContains("LogicalSort(fetch=[10])"); + } + + @Test + public void selectWithoutFrom() { + givenQuery("SELECT 1").assertPlanContains("LogicalValues(tuples=[[{ 1 }]])"); + } + + @Test + public void groupByAlias() { + givenQuery("SELECT department AS dept, COUNT(*) AS cnt FROM employees GROUP BY dept") + .assertPlanContains("LogicalAggregate(group=[{0}]"); + } + + @Test + public void groupByOrdinal() { + givenQuery("SELECT name, COUNT(*) FROM employees GROUP BY 1") + .assertPlanContains("LogicalAggregate"); + } + + @Test + public void castBooleanToInteger() { + givenQuery("SELECT CAST(true AS INTEGER) FROM employees").assertPlanContains("LogicalProject"); + } + + @Test + public void integerComparedToString() { + givenQuery("SELECT * FROM employees WHERE age > '30'").assertPlanContains("LogicalFilter"); + } + + @Test + public void matchFunction() { + givenQuery("SELECT * FROM employees WHERE match(name, 'John')") + .assertPlanContains("match(MAP('field', $1), MAP('query', 'John'))"); + } + + @Test + public void matchPhraseFunction() { + givenQuery("SELECT * FROM employees WHERE match_phrase(name, 'quick fox')") + .assertPlanContains("match_phrase(MAP('field', $1), MAP('query', 'quick fox'))"); + } + + @Test + public void namedParametersSyntax() { + givenQuery("SELECT * FROM employees WHERE match_phrase(name, 'quick fox', slop=2)") + .assertPlanContains("match_phrase(MAP('field', $1), MAP('query', 'quick fox')"); + } +} diff --git a/api/src/test/java/org/opensearch/sql/api/parser/NamedArgRewriterTest.java b/api/src/test/java/org/opensearch/sql/api/spec/search/NamedArgRewriterTest.java similarity index 98% rename from api/src/test/java/org/opensearch/sql/api/parser/NamedArgRewriterTest.java rename to api/src/test/java/org/opensearch/sql/api/spec/search/NamedArgRewriterTest.java index 32f0e8d0586..52395865548 100644 --- a/api/src/test/java/org/opensearch/sql/api/parser/NamedArgRewriterTest.java +++ b/api/src/test/java/org/opensearch/sql/api/spec/search/NamedArgRewriterTest.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package org.opensearch.sql.api.parser; +package org.opensearch.sql.api.spec.search; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; From b0be1f67c03e8256a1f5526aede03df9013bed8d Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Fri, 17 Apr 2026 11:59:43 -0700 Subject: [PATCH 2/3] refactor(api): add UnifiedPplSpec and route PPL through LanguageSpec Introduce UnifiedPplSpec as the PPL counterpart to UnifiedSqlSpec, using Calcite's default parser/validator configuration. PPL currently has its own ANTLR parser and translates directly to RelNode, so the parser and validator configs returned here are inert for the PPL path today; this lays the groundwork for post-analysis RelNode hooks. UnifiedQueryContext.Builder now selects the spec per QueryType via an exhaustive switch, removing the null branch for PPL and the now-dead null-check in buildFrameworkConfig. Signed-off-by: Chen Dai --- .../sql/api/UnifiedQueryContext.java | 21 ++++++---- .../sql/api/spec/UnifiedPplSpec.java | 42 +++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 api/src/main/java/org/opensearch/sql/api/spec/UnifiedPplSpec.java 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 8f426f4e70e..4169963da54 100644 --- a/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java +++ b/api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java @@ -29,6 +29,7 @@ import org.opensearch.sql.api.parser.PPLQueryParser; import org.opensearch.sql.api.parser.UnifiedQueryParser; import org.opensearch.sql.api.spec.LanguageSpec; +import org.opensearch.sql.api.spec.UnifiedPplSpec; import org.opensearch.sql.api.spec.UnifiedSqlSpec; import org.opensearch.sql.calcite.CalcitePlanContext; import org.opensearch.sql.calcite.SysLimit; @@ -57,7 +58,7 @@ public class UnifiedQueryContext implements AutoCloseable { /** Query parser created eagerly from this context's configuration. */ private final UnifiedQueryParser parser; - /** Language spec for SQL, or null for PPL (TODO: converge PPL onto LanguageSpec). */ + /** Language spec for the query's frontend (SQL or PPL). */ private final LanguageSpec langSpec; /** @@ -208,7 +209,11 @@ public Builder setting(String name, Object value) { public UnifiedQueryContext build() { Objects.requireNonNull(queryType, "Must specify language before build"); - LanguageSpec langSpec = (queryType == QueryType.SQL) ? UnifiedSqlSpec.extended() : null; + LanguageSpec langSpec = + switch (queryType) { + case SQL -> UnifiedSqlSpec.extended(); + case PPL -> UnifiedPplSpec.create(); + }; Settings settings = buildSettings(); CalcitePlanContext planContext = CalcitePlanContext.create( @@ -252,13 +257,11 @@ private FrameworkConfig buildFrameworkConfig(LanguageSpec langSpec) { .traitDefs((List) null) .programs(Programs.calc(DefaultRelMetadataProvider.INSTANCE)); - if (langSpec != null) { - builder - .parserConfig(langSpec.parserConfig()) - .sqlValidatorConfig(langSpec.validatorConfig()) - .operatorTable(langSpec.operatorTable()); - } - return builder.build(); + return builder + .parserConfig(langSpec.parserConfig()) + .sqlValidatorConfig(langSpec.validatorConfig()) + .operatorTable(langSpec.operatorTable()) + .build(); } private SchemaPlus findSchemaByPath(SchemaPlus rootSchema, String defaultPath) { diff --git a/api/src/main/java/org/opensearch/sql/api/spec/UnifiedPplSpec.java b/api/src/main/java/org/opensearch/sql/api/spec/UnifiedPplSpec.java new file mode 100644 index 00000000000..763f6ded540 --- /dev/null +++ b/api/src/main/java/org/opensearch/sql/api/spec/UnifiedPplSpec.java @@ -0,0 +1,42 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.api.spec; + +import java.util.List; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.validate.SqlValidator; + +/** + * PPL language specification. + * + *

Note: PPL currently has its own parsing and analyzing pipeline, so only configuration and + * extensions applied after RelNode construction are in use. The parser and validator configs + * returned here are inert for the PPL path. + */ +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class UnifiedPplSpec implements LanguageSpec { + + public static UnifiedPplSpec create() { + return new UnifiedPplSpec(); + } + + @Override + public SqlParser.Config parserConfig() { + return SqlParser.config(); + } + + @Override + public SqlValidator.Config validatorConfig() { + return SqlValidator.Config.DEFAULT; + } + + @Override + public List extensions() { + return List.of(); + } +} From 49bca0c81386ef152d6fa62feae646801ed0090e Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Fri, 17 Apr 2026 13:01:33 -0700 Subject: [PATCH 3/3] test(api): tighten lazy plan assertions and fix misplaced SHOW TABLES case Move the SHOW TABLES case out of testNonQueryStatementsBlockedByParser and into testNonQueryStatementsBlockedByWhitelist, where its error message 'Only query statements are supported' actually belongs. The parser accepts SHOW TABLES; the whitelist blocks it. Replace trivial LogicalFilter / LogicalProject / LogicalAggregate substring checks in UnifiedSqlSpecTest with assertions that prove the specific feature under test: double-quoted string literal, MATCH as a non-reserved function, GROUP BY ordinal resolution, boolean-to-integer cast folding, and BABEL's string-to-integer coercion. Use AS aliases to pin field names and avoid Calcite's EXPR$0 placeholders. Signed-off-by: Chen Dai --- .../sql/api/UnifiedQueryPlannerSqlTest.java | 11 +++-------- .../opensearch/sql/api/UnifiedSqlSpecTest.java | 16 ++++++++++------ 2 files changed, 13 insertions(+), 14 deletions(-) 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 0383ac4ec2d..855d3d2788d 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlTest.java @@ -237,6 +237,9 @@ public void testNonQueryStatementsBlockedByWhitelist() { MERGE INTO catalog.employees AS t USING (SELECT 99 AS id) AS s ON t.id = s.id WHEN MATCHED THEN UPDATE SET name = 'hacked'\ + """, + """ + SHOW TABLES\ """) .forEach( sql -> @@ -245,7 +248,6 @@ public void testNonQueryStatementsBlockedByWhitelist() { @Test public void testNonQueryStatementsBlockedByParser() { - // Babel parser rejects CREATE MATERIALIZED VIEW givenInvalidQuery( """ CREATE MATERIALIZED VIEW mv AS @@ -254,12 +256,5 @@ SELECT department, count(*) GROUP BY department\ """) .assertErrorMessage("Encountered"); - - // Babel parser accepts SHOW TABLES but it's blocked by query-type whitelist - givenInvalidQuery( - """ - SHOW TABLES\ - """) - .assertErrorMessage("Only query statements are supported"); } } 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 15868ac7530..97ddd07d0ac 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedSqlSpecTest.java @@ -48,13 +48,14 @@ public void backtickQuotedIdentifiers() { @Test public void doubleQuotedStringLiteral() { - givenQuery("SELECT \"Hello\" FROM employees").assertPlanContains("LogicalProject"); + givenQuery("SELECT \"Hello\" AS greeting FROM employees") + .assertPlanContains("LogicalProject(greeting=['Hello'])"); } @Test public void matchNotReserved() { givenQuery("SELECT * FROM employees WHERE match(name, 'Hattie')") - .assertPlanContains("LogicalFilter"); + .assertPlanContains("match(MAP('field', $1), MAP('query', 'Hattie'))"); } @Test @@ -80,18 +81,21 @@ public void groupByAlias() { @Test public void groupByOrdinal() { - givenQuery("SELECT name, COUNT(*) FROM employees GROUP BY 1") - .assertPlanContains("LogicalAggregate"); + givenQuery("SELECT name, COUNT(*) AS cnt FROM employees GROUP BY 1") + .assertPlanContains("LogicalAggregate(group=[{0}], cnt=[COUNT()])") + .assertPlanContains("LogicalProject(name=[$1])"); } @Test public void castBooleanToInteger() { - givenQuery("SELECT CAST(true AS INTEGER) FROM employees").assertPlanContains("LogicalProject"); + givenQuery("SELECT CAST(true AS INTEGER) AS val FROM employees") + .assertPlanContains("LogicalProject(val=[1])"); } @Test public void integerComparedToString() { - givenQuery("SELECT * FROM employees WHERE age > '30'").assertPlanContains("LogicalFilter"); + givenQuery("SELECT * FROM employees WHERE age > '30'") + .assertPlanContains("condition=[>($2, CAST('30'):INTEGER NOT NULL)]"); } @Test