feat(isthmus): configurable unquoted identifier casing via the ConverterProvider builder

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 @@
  * <p>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.
  *
- * <p>2. Parse the SQL query to convert (in the source SQL dialect).
+ * <p>2. Parse the SQL query to convert.
  *
  * <p>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();

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);

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.
    *
    * <p>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.
+   *
+   * <p>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}.
    *
    * <p>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<Casing> unquotedCasing = Optional.empty();
 
     // Derived from the extensions and type factory at build time when left unset.
     private Optional<ScalarFunctionConverter> 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.
+     *
+     * <p>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.

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(

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));
+  }
+}