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..33b6a53b8 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( @@ -259,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 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 */ @@ -518,8 +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)}. + * and lets callers override individual components. * * @return a new builder */ @@ -542,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(); @@ -606,6 +614,22 @@ public Builder sqlParserConfig(SqlParser.Config sqlParserConfig) { return this; } + /** + * Convenience for the common case of overriding only the unquoted-identifier casing, without + * having to restate the rest of the parser configuration. + * + *

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.unquotedCasing = Optional.ofNullable(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/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)); + } +}