feat(isthmus)!: inject ConverterProvider into the SQL statement parsers

isthmus-cli/src/main/java/io/substrait/isthmus/cli/IsthmusEntryPoint.java

@@ -96,16 +96,17 @@ public static void main(String... args) {
 
   @Override
   public Integer call() throws Exception {
+    ConverterProvider provider = converterProvider();
     // Isthmus image is parsing SQL Expression if that argument is defined
     if (sqlExpressions != null) {
-      SqlExpressionToSubstrait converter = new SqlExpressionToSubstrait(converterProvider());
+      SqlExpressionToSubstrait converter = new SqlExpressionToSubstrait(provider);
       ExtendedExpression extendedExpression = converter.convert(sqlExpressions, createStatements);
       printMessage(extendedExpression);
     } else { // by default Isthmus image are parsing SQL Query
-      SqlToSubstrait converter = new SqlToSubstrait(converterProvider());
+      SqlToSubstrait converter = new SqlToSubstrait(provider);
       Prepare.CatalogReader catalog =
           SubstraitCreateStatementParser.processCreateStatementsToCatalog(
-              createStatements.toArray(String[]::new));
+              provider, createStatements);
       Plan plan = new PlanProtoConverter().toProto(converter.convert(sql, catalog));
       printMessage(plan);
     }

isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java

@@ -70,6 +70,15 @@ public class ConverterProvider {
   /** The execution behavior configuration for plans created by this converter. */
   protected final Plan.ExecutionBehavior executionBehavior;
 
+  /**
+   * A shared default {@link ConverterProvider} instance using all system defaults. Equivalent to
+   * {@code new ConverterProvider()} but avoids redundant construction at every call site.
+   *
+   * <p>This instance is safe to share because {@link ConverterProvider} is effectively immutable
+   * after construction — all fields are set only in constructors.
+   */
+  public static final ConverterProvider DEFAULT = new ConverterProvider();
+
   /**
    * Creates a ConverterProvider with default extension collection and type factory. Uses {@link
    * DefaultExtensionCatalog#DEFAULT_COLLECTION} and {@link SubstraitTypeSystem#TYPE_FACTORY}.

isthmus/src/main/java/io/substrait/isthmus/DynamicConverterProvider.java

@@ -31,6 +31,12 @@
  */
 public class DynamicConverterProvider extends ConverterProvider {
 
+  /**
+   * The SQL operator table, computed once at construction, including operators generated from
+   * dynamic extensions. See {@link #getSqlOperatorTable()}.
+   */
+  private final SqlOperatorTable operatorTable;
+
   /**
    * Creates a new DynamicConverterProvider with default extension catalog and type factory.
    *
@@ -66,6 +72,7 @@ public DynamicConverterProvider(
       SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory) {
     super(extensions, typeFactory);
     this.scalarFunctionConverter = createScalarFunctionConverter();
+    this.operatorTable = buildSqlOperatorTable();
   }
 
   /**
@@ -109,16 +116,27 @@ public List<CallConverter> getCallConverters() {
    */
   @Override
   public SqlOperatorTable getSqlOperatorTable() {
-    SqlOperatorTable operatorTable = super.getSqlOperatorTable();
+    return operatorTable;
+  }
+
+  /**
+   * Builds the SQL operator table, chaining the base table with operators generated from dynamic
+   * extensions when any dynamic scalar or aggregate functions are present.
+   *
+   * @return the SQL operator table including standard and dynamically generated operators
+   */
+  private SqlOperatorTable buildSqlOperatorTable() {
+    SqlOperatorTable baseOperatorTable = super.getSqlOperatorTable();
     SimpleExtension.ExtensionCollection dynamicExtensionCollection =
         ExtensionUtils.getDynamicExtensions(extensions);
     if (dynamicExtensionCollection.scalarFunctions().isEmpty()
         && dynamicExtensionCollection.aggregateFunctions().isEmpty()) {
-      return operatorTable;
+      return baseOperatorTable;
     }
     List<SqlOperator> generatedDynamicOperators =
         SimpleExtensionToSqlOperator.from(dynamicExtensionCollection, typeFactory);
-    return SqlOperatorTables.chain(operatorTable, SqlOperatorTables.of(generatedDynamicOperators));
+    return SqlOperatorTables.chain(
+        baseOperatorTable, SqlOperatorTables.of(generatedDynamicOperators));
   }
 
   /**

isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java

@@ -40,7 +40,7 @@ public class SqlExpressionToSubstrait extends SqlConverterBase {
 
   /** Creates a converter with default configuration. */
   public SqlExpressionToSubstrait() {
-    this(new ConverterProvider());
+    this(ConverterProvider.DEFAULT);
   }
 
   /**
@@ -202,7 +202,7 @@ private Result registerCreateTablesForExtendedExpression(List<String> tables)
     if (tables != null) {
       for (String tableDef : tables) {
         List<SubstraitTable> tList =
-            SubstraitCreateStatementParser.processCreateStatements(tableDef);
+            SubstraitCreateStatementParser.processCreateStatements(converterProvider, tableDef);
         for (SubstraitTable t : tList) {
           rootSchema.add(t.getName(), t);
           for (RelDataTypeField field : t.getRowType(factory).getFieldList()) {
@@ -224,7 +224,8 @@ private Result registerCreateTablesForExtendedExpression(List<String> tables)
         }
       }
     }
-    SqlValidator validator = new SubstraitSqlValidator(catalogReader);
+    SqlValidator validator =
+        new SubstraitSqlValidator(catalogReader, converterProvider.getSqlOperatorTable());
     return new Result(validator, catalogReader, nameToTypeMap, nameToNodeMap);
   }
 

isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java

@@ -5,23 +5,19 @@
 import io.substrait.plan.Plan;
 import io.substrait.plan.Plan.Version;
 import org.apache.calcite.prepare.Prepare;
-import org.apache.calcite.server.ServerDdlExecutor;
 import org.apache.calcite.sql.SqlDialect;
-import org.apache.calcite.sql.SqlOperatorTable;
 import org.apache.calcite.sql.parser.SqlParseException;
-import org.apache.calcite.sql.parser.SqlParser;
 
 /**
  * Take a SQL statement and a set of table definitions and return a substrait plan.
  *
  * <p>Conversion behaviours can be customized using a {@link ConverterProvider}
  */
 public class SqlToSubstrait extends SqlConverterBase {
-  private final SqlOperatorTable operatorTable;
 
   /** Creates a SQL-to-Substrait converter using the default configuration. */
   public SqlToSubstrait() {
-    this(new ConverterProvider());
+    this(ConverterProvider.DEFAULT);
   }
 
   /**
@@ -31,7 +27,6 @@ public SqlToSubstrait() {
    */
   public SqlToSubstrait(ConverterProvider converterProvider) {
     super(converterProvider);
-    this.operatorTable = converterProvider.getSqlOperatorTable();
   }
 
   /**
@@ -50,7 +45,7 @@ public Plan convert(final String sqlStatements, final Prepare.CatalogReader cata
     builder.executionBehavior(converterProvider.getExecutionBehavior());
 
     // TODO: consider case in which one sql passes conversion while others don't
-    SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, operatorTable).stream()
+    SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, converterProvider).stream()
         .map(root -> SubstraitRelVisitor.convert(root, converterProvider))
         .forEach(root -> builder.addRoots(root));
 
@@ -60,31 +55,28 @@ public Plan convert(final String sqlStatements, final Prepare.CatalogReader cata
   /**
    * Converts one or more SQL statements into a Substrait {@link Plan}.
    *
+   * <p>The {@code sqlDialect} parameter was previously used to influence identifier casing during
+   * parsing. This is now controlled by the {@link ConverterProvider} supplied to this converter; to
+   * customise it, subclass {@link ConverterProvider} and override {@link
+   * ConverterProvider#getSqlParserConfig()}.
+   *
    * @param sqlStatements a string containing one more SQL statements
    * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in
    *     the SQL statements
    * @param sqlDialect The sql dialect to use for parsing.
    * @return the Substrait {@link Plan}
    * @throws SqlParseException if there is an error while parsing the SQL statements
+   * @deprecated Prefer constructing {@link SqlToSubstrait} with a {@link ConverterProvider}
+   *     configured for the desired casing and calling {@link #convert(String,
+   *     Prepare.CatalogReader)}. For fully custom parser behaviour, subclass {@link
+   *     ConverterProvider} and override {@link ConverterProvider#getSqlParserConfig()}.
    */
+  @Deprecated
   public Plan convert(
       final String sqlStatements,
       final Prepare.CatalogReader catalogReader,
       final SqlDialect sqlDialect)
       throws SqlParseException {
-    Builder builder = io.substrait.plan.Plan.builder();
-    builder.version(Version.builder().from(Version.DEFAULT_VERSION).producer("isthmus").build());
-    builder.executionBehavior(converterProvider.getExecutionBehavior());
-
-    final SqlParser.Config sqlParserConfig =
-        sqlDialect.configureParser(
-            SqlParser.config().withParserFactory(ServerDdlExecutor.PARSER_FACTORY));
-
-    // TODO: consider case in which one sql passes conversion while others don't
-    SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, sqlParserConfig).stream()
-        .map(root -> SubstraitRelVisitor.convert(root, converterProvider))
-        .forEach(root -> builder.addRoots(root));
-
-    return builder.build();
+    return convert(sqlStatements, catalogReader);
   }
 }

isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitCreateStatementParser.java

@@ -1,5 +1,6 @@
 package io.substrait.isthmus.sql;
 
+import io.substrait.isthmus.ConverterProvider;
 import io.substrait.isthmus.SqlConverterBase;
 import io.substrait.isthmus.SubstraitTypeSystem;
 import io.substrait.isthmus.Utils;
@@ -50,9 +51,29 @@ public class SubstraitCreateStatementParser {
    */
   public static List<SubstraitTable> processCreateStatements(@NonNull final String createStatements)
       throws SqlParseException {
+    return processCreateStatements(ConverterProvider.DEFAULT, createStatements);
+  }
+
+  /**
+   * Parses a SQL string containing only CREATE statements into a list of {@link SubstraitTable}s,
+   * using the parser settings from the given {@link ConverterProvider}.
+   *
+   * <p>This method only supports simple table names without any additional qualifiers. Only used
+   * with {@link io.substrait.isthmus.SqlExpressionToSubstrait}.
+   *
+   * @param converterProvider the converter provider whose parser config controls identifier casing
+   *     and other parser settings
+   * @param createStatements a SQL string containing only CREATE statements; must not be null
+   * @return list of {@link SubstraitTable}s generated from the CREATE statements
+   * @throws SqlParseException if parsing fails or statements are invalid
+   */
+  public static List<SubstraitTable> processCreateStatements(
+      @NonNull final ConverterProvider converterProvider, @NonNull final String createStatements)
+      throws SqlParseException {
     final List<SubstraitTable> tableList = new ArrayList<>();
 
-    final List<SqlNode> sqlNode = SubstraitSqlStatementParser.parseStatements(createStatements);
+    final List<SqlNode> sqlNode =
+        SubstraitSqlStatementParser.parseStatements(createStatements, converterProvider);
     for (final SqlNode parsed : sqlNode) {
       if (!(parsed instanceof SqlCreateTable)) {
         throw fail("Not a valid CREATE TABLE statement.");
@@ -89,6 +110,26 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
     return processCreateStatementsToCatalog(createStatements.toArray(new String[0]));
   }
 
+  /**
+   * Parses one or more SQL strings containing only CREATE statements into a {@link
+   * CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
+   *
+   * <p>This method expects the use of fully qualified table names in the CREATE statements.
+   *
+   * @param converterProvider the converter provider whose parser config controls identifier casing
+   *     and other parser settings
+   * @param createStatements List of SQL strings containing only CREATE statements, must not be null
+   * @return a {@link CalciteCatalogReader} generated from the CREATE statements
+   * @throws SqlParseException if there is an error parsing the SQL statements
+   */
+  public static CalciteCatalogReader processCreateStatementsToCatalog(
+      @NonNull final ConverterProvider converterProvider,
+      @NonNull final List<String> createStatements)
+      throws SqlParseException {
+    return processCreateStatementsToCatalog(
+        converterProvider, createStatements.toArray(new String[0]));
+  }
+
   /**
    * Parses one or more SQL strings containing only CREATE statements into a {@link
    * CalciteCatalogReader}
@@ -101,7 +142,33 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
    */
   public static CalciteCatalogReader processCreateStatementsToCatalog(
       @NonNull final String... createStatements) throws SqlParseException {
-    final CalciteSchema rootSchema = processCreateStatementsToSchema(createStatements);
+    final CalciteSchema rootSchema =
+        processCreateStatementsToSchema(ConverterProvider.DEFAULT, createStatements);
+    final List<String> defaultSchema = Collections.emptyList();
+    return new CalciteCatalogReader(
+        rootSchema,
+        defaultSchema,
+        SubstraitTypeSystem.TYPE_FACTORY,
+        SqlConverterBase.CONNECTION_CONFIG);
+  }
+
+  /**
+   * Parses one or more SQL strings containing only CREATE statements into a {@link
+   * CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
+   *
+   * <p>This method expects the use of fully qualified table names in the CREATE statements.
+   *
+   * @param converterProvider the converter provider whose parser config controls identifier casing
+   *     and other parser settings
+   * @param createStatements a SQL string containing only CREATE statements, must not be null
+   * @return a {@link CalciteCatalogReader} generated from the CREATE statements
+   * @throws SqlParseException if parsing fails or statements are invalid
+   */
+  public static CalciteCatalogReader processCreateStatementsToCatalog(
+      @NonNull final ConverterProvider converterProvider, @NonNull final String... createStatements)
+      throws SqlParseException {
+    final CalciteSchema rootSchema =
+        processCreateStatementsToSchema(converterProvider, createStatements);
     final List<String> defaultSchema = Collections.emptyList();
     return new CalciteCatalogReader(
         rootSchema,
@@ -133,19 +200,17 @@ private static SqlParseException fail(@Nullable final String message) {
   }
 
   /**
-   * Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}.
-   *
-   * @param createStatements one or more SQL strings containing only CREATE statements; must not be
-   *     null
-   * @return a {@link CalciteSchema} generated from the CREATE statements
-   * @throws SqlParseException if parsing fails or statements are invalid
+   * Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}
+   * using the given provider's parser config.
    */
   private static CalciteSchema processCreateStatementsToSchema(
-      @NonNull final String... createStatements) throws SqlParseException {
+      @NonNull final ConverterProvider converterProvider, @NonNull final String... createStatements)
+      throws SqlParseException {
     final CalciteSchema rootSchema = CalciteSchema.createRootSchema(false);
 
     for (final String statement : createStatements) {
-      final List<SqlNode> sqlNode = SubstraitSqlStatementParser.parseStatements(statement);
+      final List<SqlNode> sqlNode =
+          SubstraitSqlStatementParser.parseStatements(statement, converterProvider);
       for (final SqlNode parsed : sqlNode) {
         if (!(parsed instanceof SqlCreateTable)) {
           throw fail("Not a valid CREATE TABLE statement.");

isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java

@@ -1,27 +1,17 @@
 package io.substrait.isthmus.sql;
 
+import io.substrait.isthmus.ConverterProvider;
 import java.util.List;
-import org.apache.calcite.avatica.util.Casing;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParser;
-import org.apache.calcite.sql.parser.ddl.SqlDdlParserImpl;
-import org.apache.calcite.sql.validate.SqlConformanceEnum;
 
 /**
  * Utility class for parsing SQL statements to {@link SqlNode}s using a Substrait flavoured SQL
  * parser. Intended for testing and experimentation.
  */
 public class SubstraitSqlStatementParser {
 
-  private static final SqlParser.Config PARSER_CONFIG =
-      SqlParser.config()
-          // TODO: switch to Casing.UNCHANGED
-          .withUnquotedCasing(Casing.TO_UPPER)
-          // use LENIENT conformance to allow for parsing a wide variety of dialects
-          .withConformance(SqlConformanceEnum.LENIENT)
-          .withParserFactory(SqlDdlParserImpl.FACTORY);
-
   /**
    * Parse one or more SQL statements to a list of {@link SqlNode}s.
    *
@@ -30,20 +20,25 @@ public class SubstraitSqlStatementParser {
    * @throws SqlParseException if there is an error while parsing the SQL statements
    */
   public static List<SqlNode> parseStatements(String sqlStatements) throws SqlParseException {
-    return parseStatements(sqlStatements, PARSER_CONFIG);
+    return parseStatements(sqlStatements, ConverterProvider.DEFAULT);
   }
 
   /**
-   * Parse one or more SQL statements to a list of {@link SqlNode}s.
+   * Parse one or more SQL statements to a list of {@link SqlNode}s, using the parser settings from
+   * the given {@link ConverterProvider}.
+   *
+   * <p>To use a custom parser configuration, subclass {@link ConverterProvider} and override {@link
+   * ConverterProvider#getSqlParserConfig()}.
    *
    * @param sqlStatements a string containing one or more SQL statements
-   * @param parserConfig Calcite SqlParser.Config to control the parser
+   * @param converterProvider the converter provider whose parser config controls identifier casing
+   *     and other parser settings
    * @return a list of {@link SqlNode}s corresponding to the given statements
    * @throws SqlParseException if there is an error while parsing the SQL statements
    */
-  public static List<SqlNode> parseStatements(String sqlStatements, SqlParser.Config parserConfig)
-      throws SqlParseException {
-    SqlParser parser = SqlParser.create(sqlStatements, parserConfig);
+  public static List<SqlNode> parseStatements(
+      String sqlStatements, ConverterProvider converterProvider) throws SqlParseException {
+    SqlParser parser = SqlParser.create(sqlStatements, converterProvider.getSqlParserConfig());
     return parser.parseStmtList();
   }
 }