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 03d4fef2b..22efaaab5 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 @@ -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); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java index 8cd3731b3..f8b4b130e 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/ConverterProvider.java +++ b/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. + * + *

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}. diff --git a/isthmus/src/main/java/io/substrait/isthmus/DynamicConverterProvider.java b/isthmus/src/main/java/io/substrait/isthmus/DynamicConverterProvider.java index 8719ea9d4..39d47b504 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/DynamicConverterProvider.java +++ b/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 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 generatedDynamicOperators = SimpleExtensionToSqlOperator.from(dynamicExtensionCollection, typeFactory); - return SqlOperatorTables.chain(operatorTable, SqlOperatorTables.of(generatedDynamicOperators)); + return SqlOperatorTables.chain( + baseOperatorTable, SqlOperatorTables.of(generatedDynamicOperators)); } /** diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java index 5c020d5d1..af5b6c855 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlExpressionToSubstrait.java +++ b/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 tables) if (tables != null) { for (String tableDef : tables) { List 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 tables) } } } - SqlValidator validator = new SubstraitSqlValidator(catalogReader); + SqlValidator validator = + new SubstraitSqlValidator(catalogReader, converterProvider.getSqlOperatorTable()); return new Result(validator, catalogReader, nameToTypeMap, nameToNodeMap); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java index 765dd8dfd..284572de9 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SqlToSubstrait.java @@ -5,11 +5,8 @@ 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. @@ -17,11 +14,10 @@ *

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}. * + *

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); } } diff --git a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitCreateStatementParser.java b/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitCreateStatementParser.java index b008d577d..3d62e2726 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitCreateStatementParser.java +++ b/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 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}. + * + *

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 processCreateStatements( + @NonNull final ConverterProvider converterProvider, @NonNull final String createStatements) + throws SqlParseException { final List tableList = new ArrayList<>(); - final List sqlNode = SubstraitSqlStatementParser.parseStatements(createStatements); + final List 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}. + * + *

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 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 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}. + * + *

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 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 = SubstraitSqlStatementParser.parseStatements(statement); + final List sqlNode = + SubstraitSqlStatementParser.parseStatements(statement, converterProvider); for (final SqlNode parsed : sqlNode) { if (!(parsed instanceof SqlCreateTable)) { throw fail("Not a valid CREATE TABLE statement."); diff --git a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java b/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java index 8da220c84..a7cd777e6 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java +++ b/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlStatementParser.java @@ -1,12 +1,10 @@ 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 @@ -14,14 +12,6 @@ */ 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 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}. + * + *

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 parseStatements(String sqlStatements, SqlParser.Config parserConfig) - throws SqlParseException { - SqlParser parser = SqlParser.create(sqlStatements, parserConfig); + public static List parseStatements( + String sqlStatements, ConverterProvider converterProvider) throws SqlParseException { + SqlParser parser = SqlParser.create(sqlStatements, converterProvider.getSqlParserConfig()); return parser.parseStmtList(); } } diff --git a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlToCalcite.java b/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlToCalcite.java index d2ec84111..82650291b 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlToCalcite.java +++ b/isthmus/src/main/java/io/substrait/isthmus/sql/SubstraitSqlToCalcite.java @@ -1,5 +1,6 @@ package io.substrait.isthmus.sql; +import io.substrait.isthmus.ConverterProvider; import io.substrait.isthmus.SubstraitTypeSystem; import io.substrait.isthmus.calcite.rel.DdlSqlToRelConverter; import java.util.List; @@ -17,7 +18,6 @@ import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlOperatorTable; import org.apache.calcite.sql.parser.SqlParseException; -import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql.validate.SqlValidator; import org.apache.calcite.sql2rel.SqlToRelConverter; import org.apache.calcite.sql2rel.StandardConvertletTable; @@ -52,7 +52,12 @@ public static RelRoot convertQuery(String sqlStatement, Prepare.CatalogReader ca * @param operatorTable the {@link SqlOperatorTable} for controlling valid operators * @return a {@link RelRoot} corresponding to the given SQL statement * @throws SqlParseException if there is an error while parsing the SQL statement + * @deprecated Prefer {@link #convertQuery(String, Prepare.CatalogReader, ConverterProvider)}: the + * operator table now comes from the {@link ConverterProvider}, which also controls the parser + * configuration. To use a custom operator table, subclass {@link ConverterProvider} and + * override {@link ConverterProvider#getSqlOperatorTable()}. */ + @Deprecated public static RelRoot convertQuery( String sqlStatement, Prepare.CatalogReader catalogReader, SqlOperatorTable operatorTable) throws SqlParseException { @@ -60,6 +65,28 @@ public static RelRoot convertQuery( return convertQuery(sqlStatement, catalogReader, validator, createDefaultRelOptCluster()); } + /** + * Converts a SQL statement to a Calcite {@link RelRoot}, using the parser configuration and + * operator table from the given {@link ConverterProvider}. + * + * @param sqlStatement a SQL statement string + * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in + * the SQL statement + * @param converterProvider the converter provider whose parser config controls identifier casing + * and other parser settings, and whose {@link ConverterProvider#getSqlOperatorTable()} + * controls the valid operators + * @return a {@link RelRoot} corresponding to the given SQL statement + * @throws SqlParseException if there is an error while parsing the SQL statement + */ + public static RelRoot convertQuery( + String sqlStatement, Prepare.CatalogReader catalogReader, ConverterProvider converterProvider) + throws SqlParseException { + SqlValidator validator = + new SubstraitSqlValidator(catalogReader, converterProvider.getSqlOperatorTable()); + return convertQuery( + sqlStatement, catalogReader, validator, createDefaultRelOptCluster(), converterProvider); + } + /** * Converts a SQL statement to a Calcite {@link RelRoot}. * @@ -81,7 +108,18 @@ public static RelRoot convertQuery( SqlValidator validator, RelOptCluster cluster) throws SqlParseException { - List sqlNodes = SubstraitSqlStatementParser.parseStatements(sqlStatement); + return convertQuery(sqlStatement, catalogReader, validator, cluster, ConverterProvider.DEFAULT); + } + + static RelRoot convertQuery( + String sqlStatement, + Prepare.CatalogReader catalogReader, + SqlValidator validator, + RelOptCluster cluster, + ConverterProvider converterProvider) + throws SqlParseException { + List sqlNodes = + SubstraitSqlStatementParser.parseStatements(sqlStatement, converterProvider); if (sqlNodes.size() != 1) { throw new IllegalArgumentException( String.format("Expected one statement, found: %d", sqlNodes.size())); @@ -101,7 +139,12 @@ public static RelRoot convertQuery( * @param operatorTable the {@link SqlOperatorTable} for controlling valid operators * @return a list of {@link RelRoot}s corresponding to the given SQL statements * @throws SqlParseException if there is an error while parsing the SQL statements + * @deprecated Prefer {@link #convertQueries(String, Prepare.CatalogReader, ConverterProvider)}: + * the operator table now comes from the {@link ConverterProvider}, which also controls the + * parser configuration. To use a custom operator table, subclass {@link ConverterProvider} + * and override {@link ConverterProvider#getSqlOperatorTable()}. */ + @Deprecated public static List convertQueries( String sqlStatements, Prepare.CatalogReader catalogReader, SqlOperatorTable operatorTable) throws SqlParseException { @@ -111,18 +154,27 @@ public static List convertQueries( /** * Converts one or more SQL statements to a List of {@link RelRoot}, with one {@link RelRoot} per - * statement. + * statement, using the parser configuration and operator table from the given {@link + * ConverterProvider}. * * @param sqlStatements a string containing one or more SQL statements * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in * the SQL statements + * @param converterProvider the converter provider whose parser config controls identifier casing + * and other parser settings, and whose {@link ConverterProvider#getSqlOperatorTable()} + * controls the valid operators * @return a list of {@link RelRoot}s corresponding to the given SQL statements * @throws SqlParseException if there is an error while parsing the SQL statements */ public static List convertQueries( - String sqlStatements, Prepare.CatalogReader catalogReader) throws SqlParseException { - SqlValidator validator = new SubstraitSqlValidator(catalogReader); - return convertQueries(sqlStatements, catalogReader, validator, createDefaultRelOptCluster()); + String sqlStatements, + Prepare.CatalogReader catalogReader, + ConverterProvider converterProvider) + throws SqlParseException { + SqlValidator validator = + new SubstraitSqlValidator(catalogReader, converterProvider.getSqlOperatorTable()); + return convertQueries( + sqlStatements, catalogReader, validator, createDefaultRelOptCluster(), converterProvider); } /** @@ -132,18 +184,13 @@ public static List convertQueries( * @param sqlStatements a string containing one or more SQL statements * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in * the SQL statements - * @param parserConfig Calcite Parser config to use with the given SQL Statements * @return a list of {@link RelRoot}s corresponding to the given SQL statements * @throws SqlParseException if there is an error while parsing the SQL statements */ public static List convertQueries( - String sqlStatements, - Prepare.CatalogReader catalogReader, - final SqlParser.Config parserConfig) - throws SqlParseException { + String sqlStatements, Prepare.CatalogReader catalogReader) throws SqlParseException { SqlValidator validator = new SubstraitSqlValidator(catalogReader); - return convertQueries( - sqlStatements, catalogReader, validator, createDefaultRelOptCluster(), parserConfig); + return convertQueries(sqlStatements, catalogReader, validator, createDefaultRelOptCluster()); } /** @@ -168,36 +215,19 @@ public static List convertQueries( SqlValidator validator, RelOptCluster cluster) throws SqlParseException { - List sqlNodes = SubstraitSqlStatementParser.parseStatements(sqlStatements); - return convert(sqlNodes, catalogReader, validator, cluster); + return convertQueries( + sqlStatements, catalogReader, validator, cluster, ConverterProvider.DEFAULT); } - /** - * Converts one or more SQL statements to a List of {@link RelRoot}, with one {@link RelRoot} per - * statement. - * - * @param sqlStatements a string containing one or more SQL statements - * @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in - * the SQL statements - * @param validator the {@link SqlValidator} used to validate SQL statements. Allows for - * additional control of SQL functions and operators via {@link - * SqlValidator#getOperatorTable()} - * @param cluster the {@link RelOptCluster} used when creating {@link RelNode}s during statement - * processing. Calcite expects that the {@link RelOptCluster} used during statement processing - * is the same as that used during query optimization. - * @param parserConfig Calcite Parser config to use with the given SQL Statements - * @return a list of {@link RelRoot}s corresponding to the given SQL statements - * @throws SqlParseException if there is an error while parsing the SQL statements - */ - public static List convertQueries( + static List convertQueries( String sqlStatements, Prepare.CatalogReader catalogReader, SqlValidator validator, RelOptCluster cluster, - SqlParser.Config parserConfig) + ConverterProvider converterProvider) throws SqlParseException { List sqlNodes = - SubstraitSqlStatementParser.parseStatements(sqlStatements, parserConfig); + SubstraitSqlStatementParser.parseStatements(sqlStatements, converterProvider); return convert(sqlNodes, catalogReader, validator, cluster); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionTest.java b/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionTest.java index fbd6a4163..760a2fc03 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionTest.java @@ -2,7 +2,6 @@ import io.substrait.isthmus.sql.SubstraitCreateStatementParser; import org.apache.calcite.prepare.Prepare; -import org.apache.calcite.sql.SqlDialect; import org.apache.calcite.sql.parser.SqlParseException; import org.junit.jupiter.api.Test; @@ -14,9 +13,6 @@ void testConversion() throws SqlParseException { "create table src1 (intcol int, charcol varchar(10))"); final SqlToSubstrait converter = new SqlToSubstrait(); - converter.convert( - "create table dst1 as select * from src1", - catalogReader, - SqlDialect.DatabaseProduct.CALCITE.getDialect()); + converter.convert("create table dst1 as select * from src1", catalogReader); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionWithOptimizationTest.java b/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionWithOptimizationTest.java index 68763615c..12f381119 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionWithOptimizationTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/DdlToSubstraitConversionWithOptimizationTest.java @@ -19,10 +19,7 @@ import org.apache.calcite.rel.RelRoot; import org.apache.calcite.rel.core.Join; import org.apache.calcite.rel.rules.CoreRules; -import org.apache.calcite.server.ServerDdlExecutor; -import org.apache.calcite.sql.SqlDialect; import org.apache.calcite.sql.parser.SqlParseException; -import org.apache.calcite.sql.parser.SqlParser; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -36,13 +33,7 @@ */ class DdlToSubstraitConversionWithOptimizationTest { - /** Parser configuration for DDL statements. */ - private static final SqlParser.Config DDL_PARSER_CONFIG = - SqlDialect.DatabaseProduct.CALCITE - .getDialect() - .configureParser(SqlParser.config().withParserFactory(ServerDdlExecutor.PARSER_FACTORY)); - - private final ConverterProvider converterProvider = new ConverterProvider(); + private final ConverterProvider converterProvider = ConverterProvider.DEFAULT; /** * Tests that optimization rules can be safely applied to DDL statements. @@ -68,7 +59,7 @@ void testDdlOptimizationMutation(String ddlType) throws SqlParseException { // Convert DDL to RelRoot List relRoots = - SubstraitSqlToCalcite.convertQueries(createStatement, catalogReader, DDL_PARSER_CONFIG); + SubstraitSqlToCalcite.convertQueries(createStatement, catalogReader, converterProvider); RelRoot relRoot = relRoots.get(0); // Apply an optimization rule (FILTER_INTO_JOIN) that will structurally mutate the children diff --git a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java index afbc878a7..7686f5e0b 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java +++ b/isthmus/src/test/java/io/substrait/isthmus/PlanTestBase.java @@ -245,8 +245,7 @@ protected void assertFullRoundTrip(String sqlQuery, Prepare.CatalogReader catalo // SQL -> Calcite 1 RelRoot calcite1 = - SubstraitSqlToCalcite.convertQuery( - sqlQuery, catalogReader, converterProvider.getSqlOperatorTable()); + SubstraitSqlToCalcite.convertQuery(sqlQuery, catalogReader, converterProvider); // Calcite 1 -> Substrait POJO 1 Plan.Root root1 = SubstraitRelVisitor.convert(calcite1, converterProvider); diff --git a/isthmus/src/test/java/io/substrait/isthmus/StatisticalFunctionTest.java b/isthmus/src/test/java/io/substrait/isthmus/StatisticalFunctionTest.java index 286ebfe6f..9cef0f66f 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/StatisticalFunctionTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/StatisticalFunctionTest.java @@ -80,9 +80,7 @@ void usesEnumArgSignature(String sqlFn, String substraitFn, String distribution) SubstraitCreateStatementParser.processCreateStatementsToCatalog(CREATES); RelRoot calcite = SubstraitSqlToCalcite.convertQuery( - String.format("SELECT %s(fp64) FROM numbers", sqlFn), - catalog, - converterProvider.getSqlOperatorTable()); + String.format("SELECT %s(fp64) FROM numbers", sqlFn), catalog, converterProvider); Plan.Root root = SubstraitRelVisitor.convert(calcite, converterProvider); AggregateFunctionInvocation function = firstMeasure(root.getInput()).getFunction();