From 130e81fcc328d1e7d58598e85e9aa7d822e1fb76 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Fri, 24 Jul 2026 08:21:25 +0200 Subject: [PATCH 1/3] feat(isthmus): add unquoted-casing toggling to the ConverterProvider builder Add Builder.unquotedCasing(Casing) as a convenience for the common case of overriding only the unquoted-identifier casing, layered on top of the current parser configuration. Route the isthmus CLI's --unquotedcasing option through it, use it in the FromSql example (Casing.UNCHANGED, matching the lower-case identifiers in the example's CREATE TABLE statements), and add UnquotedCasingTest covering the default casing, the convenience for each Casing, the full sqlParserConfig path (retaining LENIENT conformance), and end-to-end NamedScan naming (EMPLOYEES under TO_UPPER, employees under UNCHANGED). --- .../java/io/substrait/examples/FromSql.java | 24 +++-- .../isthmus/cli/IsthmusEntryPoint.java | 6 +- .../substrait/isthmus/ConverterProvider.java | 24 ++++- .../substrait/isthmus/UnquotedCasingTest.java | 94 +++++++++++++++++++ 4 files changed, 131 insertions(+), 17 deletions(-) create mode 100644 isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java diff --git a/examples/isthmus-api/src/main/java/io/substrait/examples/FromSql.java b/examples/isthmus-api/src/main/java/io/substrait/examples/FromSql.java index 17973b683..15f179f60 100644 --- a/examples/isthmus-api/src/main/java/io/substrait/examples/FromSql.java +++ b/examples/isthmus-api/src/main/java/io/substrait/examples/FromSql.java @@ -1,6 +1,7 @@ package io.substrait.examples; import io.substrait.examples.IsthmusAppExamples.Action; +import io.substrait.isthmus.ConverterProvider; import io.substrait.isthmus.SqlToSubstrait; import io.substrait.isthmus.sql.SubstraitCreateStatementParser; import io.substrait.plan.Plan; @@ -10,8 +11,8 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import org.apache.calcite.avatica.util.Casing; import org.apache.calcite.prepare.CalciteCatalogReader; -import org.apache.calcite.sql.SqlDialect; import org.apache.calcite.sql.parser.SqlParseException; /** @@ -22,7 +23,7 @@ *

1. Create a fully typed schema for the inputs. Within a SQL context this represents the CREATE * TABLE commands, which need to be converted to a Calcite Schema. * - *

2. Parse the SQL query to convert (in the source SQL dialect). + *

2. Parse the SQL query to convert. * *

3. Convert the SQL query to Calcite Relations. * @@ -49,22 +50,27 @@ public void run(final String[] args) { "test_result" varchar(15),"test_mileage" int, "postcode_area" varchar(15)); """); + // The unquoted identifier casing applied while parsing is configurable via a + // ConverterProvider. The same provider is used for both the schema and the query so that + // identifier casing stays consistent end-to-end. Casing.UNCHANGED preserves identifiers as + // written, matching the lower-case names used in the CREATE TABLE statements above. + final ConverterProvider converterProvider = + ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); + final CalciteCatalogReader catalogReader = - SubstraitCreateStatementParser.processCreateStatementsToCatalog(createSqlStatements); + SubstraitCreateStatementParser.processCreateStatementsToCatalog( + converterProvider, createSqlStatements); - // Query that needs to be converted; again this could be in a variety of SQL - // dialects + // Query that needs to be converted final String sqlQuery = """ SELECT vehicles.colour, count(*) as colourcount FROM vehicles INNER JOIN tests ON vehicles.vehicle_id=tests.vehicle_id WHERE tests.test_result = 'P' GROUP BY vehicles.colour ORDER BY count(*) """; - final SqlToSubstrait sqlToSubstrait = new SqlToSubstrait(); - // choose DuckDB as an example dialect - final SqlDialect dialect = SqlDialect.DatabaseProduct.DUCKDB.getDialect(); - final Plan substraitPlan = sqlToSubstrait.convert(sqlQuery, catalogReader, dialect); + final SqlToSubstrait sqlToSubstrait = new SqlToSubstrait(converterProvider); + final Plan substraitPlan = sqlToSubstrait.convert(sqlQuery, catalogReader); // Create the proto plan to display to stdout - as it has a better format final PlanProtoConverter planToProto = new PlanProtoConverter(); diff --git a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java index b80bec415..8c4a40098 100644 --- a/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java +++ b/isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java @@ -86,11 +86,7 @@ public static void main(String... args) { @Override public Integer call() throws Exception { - ConverterProvider provider = - ConverterProvider.builder() - .sqlParserConfig( - ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.withUnquotedCasing(unquotedCasing)) - .build(); + ConverterProvider provider = ConverterProvider.builder().unquotedCasing(unquotedCasing).build(); // Isthmus image is parsing SQL Expression if that argument is defined if (sqlExpressions != null) { SqlExpressionToSubstrait converter = new SqlExpressionToSubstrait(provider); diff --git a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java index f6e126358..1765907b4 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java +++ b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java @@ -259,8 +259,8 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() { * identifier casing. * *

Defaults to {@link #DEFAULT_SQL_PARSER_CONFIG}. Provide a custom configuration via {@link - * Builder#sqlParserConfig(SqlParser.Config)}, or override this method in a subclass for fully - * dynamic behaviour. + * Builder#sqlParserConfig(SqlParser.Config)} (or the {@link Builder#unquotedCasing(Casing)} + * convenience), or override this method in a subclass for fully dynamic behaviour. * * @return the SQL parser configuration */ @@ -519,7 +519,8 @@ public Plan.ExecutionBehavior getExecutionBehavior() { * *

The builder starts from reasonable system defaults (the same ones behind {@link #DEFAULT}) * and lets callers override individual components — most notably the Calcite {@link - * SqlParser.Config} used for SQL parsing, via {@link Builder#sqlParserConfig(SqlParser.Config)}. + * SqlParser.Config} used for SQL parsing, via {@link Builder#sqlParserConfig(SqlParser.Config)} + * for full control or {@link Builder#unquotedCasing(Casing)} for the common casing-only case. * * @return a new builder */ @@ -606,6 +607,23 @@ public Builder sqlParserConfig(SqlParser.Config sqlParserConfig) { return this; } + /** + * Convenience for the common case of overriding only the unquoted-identifier casing, applied on + * top of the current {@link #sqlParserConfig(SqlParser.Config) parser configuration}. + * + *

Equivalent to {@code sqlParserConfig(currentConfig.withUnquotedCasing(unquotedCasing))}. + * Because it layers onto the current configuration, a subsequent {@link + * #sqlParserConfig(SqlParser.Config)} call replaces the whole configuration and discards the + * casing set here; set the full config first, then apply this convenience. + * + * @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing + * @return this builder + */ + public Builder unquotedCasing(Casing unquotedCasing) { + this.sqlParserConfig = this.sqlParserConfig.withUnquotedCasing(unquotedCasing); + return this; + } + /** * Sets the scalar function converter. When left unset, it is derived from the configured * extensions and type factory. diff --git a/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java b/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java new file mode 100644 index 000000000..3e19c6708 --- /dev/null +++ b/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java @@ -0,0 +1,94 @@ +package io.substrait.isthmus; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.substrait.isthmus.sql.SubstraitCreateStatementParser; +import io.substrait.plan.Plan; +import io.substrait.relation.NamedScan; +import io.substrait.relation.Project; +import org.apache.calcite.avatica.util.Casing; +import org.apache.calcite.prepare.Prepare; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.validate.SqlConformanceEnum; +import org.junit.jupiter.api.Test; + +/** + * Verifies that {@link ConverterProvider#builder()} configures the Calcite {@link SqlParser.Config} + * used for SQL parsing — via the {@code unquotedCasing} convenience or a full {@code + * sqlParserConfig} — and that the configured casing is applied consistently across both CREATE + * TABLE parsing and query parsing, so that the table name stored in a Substrait {@link NamedScan} + * reflects the configured casing. + */ +class UnquotedCasingTest { + + private static final String CREATE_STATEMENT = "CREATE TABLE employees (id BIGINT, name VARCHAR)"; + + @Test + void defaultCasingIsToUpper() { + ConverterProvider provider = ConverterProvider.builder().build(); + assertEquals(Casing.TO_UPPER, provider.getSqlParserConfig().unquotedCasing()); + } + + @Test + void builderCasingUnchanged() { + ConverterProvider provider = + ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); + assertEquals(Casing.UNCHANGED, provider.getSqlParserConfig().unquotedCasing()); + } + + @Test + void builderCasingToLower() { + ConverterProvider provider = + ConverterProvider.builder().unquotedCasing(Casing.TO_LOWER).build(); + assertEquals(Casing.TO_LOWER, provider.getSqlParserConfig().unquotedCasing()); + } + + /** + * A full {@link SqlParser.Config} supplied to the builder is used verbatim. Deriving it from + * {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG} preserves Isthmus' parser defaults (here, + * {@link SqlConformanceEnum#LENIENT} conformance) while overriding a single setting. + */ + @Test + void builderFullSqlParserConfig() { + SqlParser.Config config = + ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.withUnquotedCasing(Casing.TO_LOWER); + ConverterProvider provider = ConverterProvider.builder().sqlParserConfig(config).build(); + assertEquals(Casing.TO_LOWER, provider.getSqlParserConfig().unquotedCasing()); + assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); + } + + /** + * With the default {@link Casing#TO_UPPER}, both CREATE TABLE and SELECT fold unquoted + * identifiers to upper-case. The resulting {@link NamedScan} table name is {@code EMPLOYEES}. + */ + @Test + void defaultCasingFoldsTableNameToUpper() throws Exception { + ConverterProvider provider = ConverterProvider.builder().build(); + Prepare.CatalogReader catalog = + SubstraitCreateStatementParser.processCreateStatementsToCatalog(provider, CREATE_STATEMENT); + + Plan plan = new SqlToSubstrait(provider).convert("SELECT id FROM employees", catalog); + + NamedScan scan = (NamedScan) ((Project) plan.getRoots().get(0).getInput()).getInput(); + assertEquals("EMPLOYEES", scan.getNames().get(0)); + } + + /** + * With {@link Casing#UNCHANGED}, both CREATE TABLE and SELECT preserve the written casing of + * unquoted identifiers. A query using lowercase {@code employees} against a catalog built from + * {@code CREATE TABLE employees} therefore produces a {@link NamedScan} with name {@code + * employees}. + */ + @Test + void unchangedCasingPreservesTableName() throws Exception { + ConverterProvider provider = + ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); + Prepare.CatalogReader catalog = + SubstraitCreateStatementParser.processCreateStatementsToCatalog(provider, CREATE_STATEMENT); + + Plan plan = new SqlToSubstrait(provider).convert("SELECT id FROM employees", catalog); + + NamedScan scan = (NamedScan) ((Project) plan.getRoots().get(0).getInput()).getInput(); + assertEquals("employees", scan.getNames().get(0)); + } +} From cc55ddcc9cd3710bfdc917dc996553b7f8ac5813 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Mon, 3 Aug 2026 09:59:05 +0200 Subject: [PATCH 2/3] refactor(isthmus): make unquotedCasing order-independent with sqlParserConfig The convenience mutated the builder's parser config eagerly, so the result depended on call order: sqlParserConfig(y).unquotedCasing(x) produced y with x applied, while unquotedCasing(x).sqlParserConfig(y) silently discarded x. The Javadoc documented that ordering requirement rather than removing it. Hold the casing as an Optional instead and apply it over the parser config in the primary constructor. Both orders now yield the same provider, and the rule is simply that the casing override wins over whatever casing the supplied config carries. --- .../substrait/isthmus/ConverterProvider.java | 25 ++++++---- .../substrait/isthmus/UnquotedCasingTest.java | 46 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java index 1765907b4..73510b018 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java +++ b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java @@ -213,7 +213,13 @@ protected ConverterProvider(Builder builder) { this.extensions = builder.extensions; this.typeConverter = builder.typeConverter; this.executionBehavior = builder.executionBehavior; - this.sqlParserConfig = builder.sqlParserConfig; + // An unquoted casing set through the builder convenience is applied over the parser config + // here, so that the two setters are order-independent. + this.sqlParserConfig = + builder + .unquotedCasing + .map(builder.sqlParserConfig::withUnquotedCasing) + .orElse(builder.sqlParserConfig); this.scalarFunctionConverter = builder.scalarFunctionConverter.orElseGet( @@ -543,6 +549,7 @@ public static class Builder { private TypeConverter typeConverter = TypeConverter.DEFAULT; private Plan.ExecutionBehavior executionBehavior = createDefaultExecutionBehavior(); private SqlParser.Config sqlParserConfig = DEFAULT_SQL_PARSER_CONFIG; + private Optional unquotedCasing = Optional.empty(); // Derived from the extensions and type factory at build time when left unset. private Optional scalarFunctionConverter = Optional.empty(); @@ -608,19 +615,21 @@ public Builder sqlParserConfig(SqlParser.Config sqlParserConfig) { } /** - * Convenience for the common case of overriding only the unquoted-identifier casing, applied on - * top of the current {@link #sqlParserConfig(SqlParser.Config) parser configuration}. + * Convenience for the common case of overriding only the unquoted-identifier casing, without + * having to restate the rest of the parser configuration. * - *

Equivalent to {@code sqlParserConfig(currentConfig.withUnquotedCasing(unquotedCasing))}. - * Because it layers onto the current configuration, a subsequent {@link - * #sqlParserConfig(SqlParser.Config)} call replaces the whole configuration and discards the - * casing set here; set the full config first, then apply this convenience. + *

Unlike deriving a config by hand, this cannot accidentally drop the DDL parser factory or + * conformance that {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG} supplies. + * + *

The casing set here is applied over the {@link #sqlParserConfig(SqlParser.Config) parser + * configuration} when the provider is constructed, so it wins over any casing that + * configuration carries and the two setters may be called in either order. * * @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing * @return this builder */ public Builder unquotedCasing(Casing unquotedCasing) { - this.sqlParserConfig = this.sqlParserConfig.withUnquotedCasing(unquotedCasing); + this.unquotedCasing = Optional.ofNullable(unquotedCasing); return this; } diff --git a/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java b/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java index 3e19c6708..46aa15b2e 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java @@ -57,6 +57,52 @@ void builderFullSqlParserConfig() { assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); } + /** + * The {@code unquotedCasing} convenience is applied over the configured {@link SqlParser.Config} + * at construction, so it wins over the casing that config carries no matter which order the two + * setters are called in, and the rest of the supplied config is retained either way. + */ + @Test + void unquotedCasingIsOrderIndependentWithSqlParserConfig() { + SqlParser.Config config = + ConverterProvider.DEFAULT_SQL_PARSER_CONFIG + .withUnquotedCasing(Casing.TO_LOWER) + .withConformance(SqlConformanceEnum.PRAGMATIC_2003); + + ConverterProvider casingLast = + ConverterProvider.builder() + .sqlParserConfig(config) + .unquotedCasing(Casing.UNCHANGED) + .build(); + ConverterProvider casingFirst = + ConverterProvider.builder() + .unquotedCasing(Casing.UNCHANGED) + .sqlParserConfig(config) + .build(); + + assertEquals(Casing.UNCHANGED, casingLast.getSqlParserConfig().unquotedCasing()); + assertEquals(Casing.UNCHANGED, casingFirst.getSqlParserConfig().unquotedCasing()); + assertEquals(SqlConformanceEnum.PRAGMATIC_2003, casingLast.getSqlParserConfig().conformance()); + assertEquals(SqlConformanceEnum.PRAGMATIC_2003, casingFirst.getSqlParserConfig().conformance()); + } + + /** + * The convenience layers onto {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG}, so it cannot + * drop the DDL parser factory or conformance the way hand-deriving from {@link + * SqlParser.Config#DEFAULT} would. + */ + @Test + void unquotedCasingRetainsIsthmusParserDefaults() { + ConverterProvider provider = + ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); + + assertEquals(Casing.UNCHANGED, provider.getSqlParserConfig().unquotedCasing()); + assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); + assertEquals( + ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.parserFactory(), + provider.getSqlParserConfig().parserFactory()); + } + /** * With the default {@link Casing#TO_UPPER}, both CREATE TABLE and SELECT fold unquoted * identifiers to upper-case. The resulting {@link NamedScan} table name is {@code EMPLOYEES}. From c80277836d3e769ce881c25dfced7c7b493bea13 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Tue, 4 Aug 2026 08:21:29 +0200 Subject: [PATCH 3/3] refactor(isthmus): trim casing docs and regroup the casing tests Address review feedback on the unquoted-casing convenience. Docs: drop the parser-config callouts from `builder()` and the hand-derivation warning from `unquotedCasing(...)`, and mention the convenience from `getSqlParserConfig()` in a paragraph of its own rather than parenthetically. Tests: fold the builder-level assertions into `ConverterProviderBuilderTest` as a nested `UnquotedCasing` section so they read as ConverterProvider behaviour rather than general casing functionality, and collapse the redundant cases. The end-to-end check moves to `SqlToSubstraitTest`, parameterized over the casings so it states the property directly: the `NamedScan` name follows whatever casing the provider carries. --- .../substrait/isthmus/ConverterProvider.java | 13 +- .../isthmus/ConverterProviderBuilderTest.java | 66 +++++++++ .../substrait/isthmus/SqlToSubstraitTest.java | 36 +++++ .../substrait/isthmus/UnquotedCasingTest.java | 140 ------------------ 4 files changed, 107 insertions(+), 148 deletions(-) create mode 100644 isthmus/src/test/java/io/substrait/isthmus/SqlToSubstraitTest.java delete mode 100644 isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java diff --git a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java index 73510b018..33b6a53b8 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java +++ b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java @@ -265,8 +265,10 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() { * identifier casing. * *

Defaults to {@link #DEFAULT_SQL_PARSER_CONFIG}. Provide a custom configuration via {@link - * Builder#sqlParserConfig(SqlParser.Config)} (or the {@link Builder#unquotedCasing(Casing)} - * convenience), or override this method in a subclass for fully dynamic behaviour. + * Builder#sqlParserConfig(SqlParser.Config)}, or override this method in a subclass for even more + * control. + * + *

To override just the unquoted casing, consider {@link Builder#unquotedCasing(Casing)}. * * @return the SQL parser configuration */ @@ -524,9 +526,7 @@ public Plan.ExecutionBehavior getExecutionBehavior() { * Creates a new {@link Builder} for configuring a {@link ConverterProvider}. * *

The builder starts from reasonable system defaults (the same ones behind {@link #DEFAULT}) - * and lets callers override individual components — most notably the Calcite {@link - * SqlParser.Config} used for SQL parsing, via {@link Builder#sqlParserConfig(SqlParser.Config)} - * for full control or {@link Builder#unquotedCasing(Casing)} for the common casing-only case. + * and lets callers override individual components. * * @return a new builder */ @@ -618,9 +618,6 @@ public Builder sqlParserConfig(SqlParser.Config sqlParserConfig) { * Convenience for the common case of overriding only the unquoted-identifier casing, without * having to restate the rest of the parser configuration. * - *

Unlike deriving a config by hand, this cannot accidentally drop the DDL parser factory or - * conformance that {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG} supplies. - * *

The casing set here is applied over the {@link #sqlParserConfig(SqlParser.Config) parser * configuration} when the provider is constructed, so it wins over any casing that * configuration carries and the two setters may be called in either order. diff --git a/isthmus/src/test/java/io/substrait/isthmus/ConverterProviderBuilderTest.java b/isthmus/src/test/java/io/substrait/isthmus/ConverterProviderBuilderTest.java index c3d0f7ba2..eaed78393 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/ConverterProviderBuilderTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/ConverterProviderBuilderTest.java @@ -12,7 +12,11 @@ import io.substrait.isthmus.expression.AggregateFunctionConverter; import io.substrait.isthmus.expression.ScalarFunctionConverter; import io.substrait.isthmus.expression.WindowFunctionConverter; +import org.apache.calcite.avatica.util.Casing; import org.apache.calcite.rel.type.RelDataTypeFactory; +import org.apache.calcite.sql.parser.SqlParser; +import org.apache.calcite.sql.validate.SqlConformanceEnum; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; class ConverterProviderBuilderTest { @@ -94,6 +98,68 @@ void automaticDynamicProviderAcceptsBuilderWithoutFunctionConverters() { ConverterProvider.builder().extensions(EXTENSIONS))); } + @Nested + class UnquotedCasing { + + @Test + void defaultsToUpper() { + ConverterProvider provider = ConverterProvider.builder().build(); + assertEquals(Casing.TO_UPPER, provider.getSqlParserConfig().unquotedCasing()); + } + + @Test + void configuredCasingIsUsed() { + ConverterProvider provider = + ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); + assertEquals(Casing.UNCHANGED, provider.getSqlParserConfig().unquotedCasing()); + } + + /** + * A full {@link SqlParser.Config} supplied to the builder is used verbatim. Deriving it from + * {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG} preserves Isthmus' parser defaults (here, + * {@link SqlConformanceEnum#LENIENT} conformance) while overriding a single setting. + */ + @Test + void fullSqlParserConfigIsUsed() { + SqlParser.Config config = + ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.withUnquotedCasing(Casing.TO_LOWER); + ConverterProvider provider = ConverterProvider.builder().sqlParserConfig(config).build(); + assertEquals(Casing.TO_LOWER, provider.getSqlParserConfig().unquotedCasing()); + assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); + } + + /** + * The casing is applied over the configured {@link SqlParser.Config} at construction, so it + * wins over the casing that config carries no matter which order the two setters are called in, + * and the rest of the supplied config is retained either way. + */ + @Test + void isOrderIndependentWithSqlParserConfig() { + SqlParser.Config config = + ConverterProvider.DEFAULT_SQL_PARSER_CONFIG + .withUnquotedCasing(Casing.TO_LOWER) + .withConformance(SqlConformanceEnum.PRAGMATIC_2003); + + ConverterProvider casingLast = + ConverterProvider.builder() + .sqlParserConfig(config) + .unquotedCasing(Casing.UNCHANGED) + .build(); + ConverterProvider casingFirst = + ConverterProvider.builder() + .unquotedCasing(Casing.UNCHANGED) + .sqlParserConfig(config) + .build(); + + assertEquals(Casing.UNCHANGED, casingLast.getSqlParserConfig().unquotedCasing()); + assertEquals(Casing.UNCHANGED, casingFirst.getSqlParserConfig().unquotedCasing()); + assertEquals( + SqlConformanceEnum.PRAGMATIC_2003, casingLast.getSqlParserConfig().conformance()); + assertEquals( + SqlConformanceEnum.PRAGMATIC_2003, casingFirst.getSqlParserConfig().conformance()); + } + } + private static void assertRejected(ConverterProvider.Builder builder, String setterName) { IllegalArgumentException e = assertThrows( diff --git a/isthmus/src/test/java/io/substrait/isthmus/SqlToSubstraitTest.java b/isthmus/src/test/java/io/substrait/isthmus/SqlToSubstraitTest.java new file mode 100644 index 000000000..635934329 --- /dev/null +++ b/isthmus/src/test/java/io/substrait/isthmus/SqlToSubstraitTest.java @@ -0,0 +1,36 @@ +package io.substrait.isthmus; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.substrait.isthmus.sql.SubstraitCreateStatementParser; +import io.substrait.plan.Plan; +import io.substrait.relation.NamedScan; +import io.substrait.relation.Project; +import org.apache.calcite.avatica.util.Casing; +import org.apache.calcite.prepare.Prepare; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +class SqlToSubstraitTest { + + private static final String CREATE_STATEMENT = "CREATE TABLE employees (id BIGINT, name VARCHAR)"; + + /** + * The unquoted-identifier casing configured on the {@link ConverterProvider} is applied to both + * the CREATE statements and the query, so the table name carried by the resulting {@link + * NamedScan} follows it. + */ + @ParameterizedTest + @CsvSource({"TO_UPPER, EMPLOYEES", "TO_LOWER, employees", "UNCHANGED, employees"}) + void namedScanFollowsProviderUnquotedCasing(Casing casing, String expectedTableName) + throws Exception { + ConverterProvider provider = ConverterProvider.builder().unquotedCasing(casing).build(); + Prepare.CatalogReader catalog = + SubstraitCreateStatementParser.processCreateStatementsToCatalog(provider, CREATE_STATEMENT); + + Plan plan = new SqlToSubstrait(provider).convert("SELECT id FROM employees", catalog); + + NamedScan scan = (NamedScan) ((Project) plan.getRoots().get(0).getInput()).getInput(); + assertEquals(expectedTableName, scan.getNames().get(0)); + } +} diff --git a/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java b/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java deleted file mode 100644 index 46aa15b2e..000000000 --- a/isthmus/src/test/java/io/substrait/isthmus/UnquotedCasingTest.java +++ /dev/null @@ -1,140 +0,0 @@ -package io.substrait.isthmus; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import io.substrait.isthmus.sql.SubstraitCreateStatementParser; -import io.substrait.plan.Plan; -import io.substrait.relation.NamedScan; -import io.substrait.relation.Project; -import org.apache.calcite.avatica.util.Casing; -import org.apache.calcite.prepare.Prepare; -import org.apache.calcite.sql.parser.SqlParser; -import org.apache.calcite.sql.validate.SqlConformanceEnum; -import org.junit.jupiter.api.Test; - -/** - * Verifies that {@link ConverterProvider#builder()} configures the Calcite {@link SqlParser.Config} - * used for SQL parsing — via the {@code unquotedCasing} convenience or a full {@code - * sqlParserConfig} — and that the configured casing is applied consistently across both CREATE - * TABLE parsing and query parsing, so that the table name stored in a Substrait {@link NamedScan} - * reflects the configured casing. - */ -class UnquotedCasingTest { - - private static final String CREATE_STATEMENT = "CREATE TABLE employees (id BIGINT, name VARCHAR)"; - - @Test - void defaultCasingIsToUpper() { - ConverterProvider provider = ConverterProvider.builder().build(); - assertEquals(Casing.TO_UPPER, provider.getSqlParserConfig().unquotedCasing()); - } - - @Test - void builderCasingUnchanged() { - ConverterProvider provider = - ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); - assertEquals(Casing.UNCHANGED, provider.getSqlParserConfig().unquotedCasing()); - } - - @Test - void builderCasingToLower() { - ConverterProvider provider = - ConverterProvider.builder().unquotedCasing(Casing.TO_LOWER).build(); - assertEquals(Casing.TO_LOWER, provider.getSqlParserConfig().unquotedCasing()); - } - - /** - * A full {@link SqlParser.Config} supplied to the builder is used verbatim. Deriving it from - * {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG} preserves Isthmus' parser defaults (here, - * {@link SqlConformanceEnum#LENIENT} conformance) while overriding a single setting. - */ - @Test - void builderFullSqlParserConfig() { - SqlParser.Config config = - ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.withUnquotedCasing(Casing.TO_LOWER); - ConverterProvider provider = ConverterProvider.builder().sqlParserConfig(config).build(); - assertEquals(Casing.TO_LOWER, provider.getSqlParserConfig().unquotedCasing()); - assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); - } - - /** - * The {@code unquotedCasing} convenience is applied over the configured {@link SqlParser.Config} - * at construction, so it wins over the casing that config carries no matter which order the two - * setters are called in, and the rest of the supplied config is retained either way. - */ - @Test - void unquotedCasingIsOrderIndependentWithSqlParserConfig() { - SqlParser.Config config = - ConverterProvider.DEFAULT_SQL_PARSER_CONFIG - .withUnquotedCasing(Casing.TO_LOWER) - .withConformance(SqlConformanceEnum.PRAGMATIC_2003); - - ConverterProvider casingLast = - ConverterProvider.builder() - .sqlParserConfig(config) - .unquotedCasing(Casing.UNCHANGED) - .build(); - ConverterProvider casingFirst = - ConverterProvider.builder() - .unquotedCasing(Casing.UNCHANGED) - .sqlParserConfig(config) - .build(); - - assertEquals(Casing.UNCHANGED, casingLast.getSqlParserConfig().unquotedCasing()); - assertEquals(Casing.UNCHANGED, casingFirst.getSqlParserConfig().unquotedCasing()); - assertEquals(SqlConformanceEnum.PRAGMATIC_2003, casingLast.getSqlParserConfig().conformance()); - assertEquals(SqlConformanceEnum.PRAGMATIC_2003, casingFirst.getSqlParserConfig().conformance()); - } - - /** - * The convenience layers onto {@link ConverterProvider#DEFAULT_SQL_PARSER_CONFIG}, so it cannot - * drop the DDL parser factory or conformance the way hand-deriving from {@link - * SqlParser.Config#DEFAULT} would. - */ - @Test - void unquotedCasingRetainsIsthmusParserDefaults() { - ConverterProvider provider = - ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); - - assertEquals(Casing.UNCHANGED, provider.getSqlParserConfig().unquotedCasing()); - assertEquals(SqlConformanceEnum.LENIENT, provider.getSqlParserConfig().conformance()); - assertEquals( - ConverterProvider.DEFAULT_SQL_PARSER_CONFIG.parserFactory(), - provider.getSqlParserConfig().parserFactory()); - } - - /** - * With the default {@link Casing#TO_UPPER}, both CREATE TABLE and SELECT fold unquoted - * identifiers to upper-case. The resulting {@link NamedScan} table name is {@code EMPLOYEES}. - */ - @Test - void defaultCasingFoldsTableNameToUpper() throws Exception { - ConverterProvider provider = ConverterProvider.builder().build(); - Prepare.CatalogReader catalog = - SubstraitCreateStatementParser.processCreateStatementsToCatalog(provider, CREATE_STATEMENT); - - Plan plan = new SqlToSubstrait(provider).convert("SELECT id FROM employees", catalog); - - NamedScan scan = (NamedScan) ((Project) plan.getRoots().get(0).getInput()).getInput(); - assertEquals("EMPLOYEES", scan.getNames().get(0)); - } - - /** - * With {@link Casing#UNCHANGED}, both CREATE TABLE and SELECT preserve the written casing of - * unquoted identifiers. A query using lowercase {@code employees} against a catalog built from - * {@code CREATE TABLE employees} therefore produces a {@link NamedScan} with name {@code - * employees}. - */ - @Test - void unchangedCasingPreservesTableName() throws Exception { - ConverterProvider provider = - ConverterProvider.builder().unquotedCasing(Casing.UNCHANGED).build(); - Prepare.CatalogReader catalog = - SubstraitCreateStatementParser.processCreateStatementsToCatalog(provider, CREATE_STATEMENT); - - Plan plan = new SqlToSubstrait(provider).convert("SELECT id FROM employees", catalog); - - NamedScan scan = (NamedScan) ((Project) plan.getRoots().get(0).getInput()).getInput(); - assertEquals("employees", scan.getNames().get(0)); - } -}