Skip to content

Commit 648b0e1

Browse files
nielspardonnielspardon
committed
feat(isthmus): make unquoted identifier casing configurable in ConverterProvider
Add constructor-based configuration of unquoted SQL identifier casing to ConverterProvider, so that isthmus consumers can control how unquoted identifiers are cased during parsing. The default remains Casing.TO_UPPER (no behaviour change). Previously the only way to change this was to subclass ConverterProvider and override getSqlParserConfig() — as IsthmusEntryPoint already did with an anonymous class. That workaround is now replaced by a first-class constructor parameter. Changes to ConverterProvider: - unquotedCasing is a new final field, consistent with executionBehavior - getUnquotedCasing() getter - getSqlParserConfig() reads unquotedCasing instead of hard-coding TO_UPPER - new ConverterProvider(Casing) and ConverterProvider(extensions, typeFactory, Casing) for the common cases; the existing 7-arg constructor gains Casing as an 8th parameter; all narrower constructors default to Casing.TO_UPPER Propagation through the pipeline — casing is applied consistently across both CREATE TABLE parsing and query parsing: - SubstraitSqlToCalcite: new convertQueries(sql, catalog, ConverterProvider, operatorTable) overload passes getSqlParserConfig() to the statement parser - SqlToSubstrait: convert(sql, catalog) uses the ConverterProvider overload - SubstraitCreateStatementParser: new processCreateStatements(ConverterProvider, sql) and processCreateStatementsToCatalog(ConverterProvider, ...) overloads; SqlParser.Config stays an internal detail - SqlExpressionToSubstrait: uses processCreateStatements(converterProvider, tableDef) - IsthmusEntryPoint: uses new ConverterProvider(unquotedCasing); anonymous ConverterProvider subclass removed
1 parent a29fbb1 commit 648b0e1

7 files changed

Lines changed: 303 additions & 44 deletions

File tree

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import java.util.concurrent.Callable;
1616
import org.apache.calcite.avatica.util.Casing;
1717
import org.apache.calcite.prepare.Prepare;
18-
import org.apache.calcite.sql.parser.SqlParser;
1918
import picocli.CommandLine;
2019
import picocli.CommandLine.Command;
2120
import picocli.CommandLine.Option;
@@ -61,12 +60,7 @@ enum OutputFormat {
6160
private Casing unquotedCasing = Casing.TO_UPPER;
6261

6362
private ConverterProvider converterProvider() {
64-
return new ConverterProvider() {
65-
@Override
66-
public SqlParser.Config getSqlParserConfig() {
67-
return super.getSqlParserConfig().withUnquotedCasing(unquotedCasing);
68-
}
69-
};
63+
return new ConverterProvider(unquotedCasing);
7064
}
7165

7266
/**
@@ -102,10 +96,11 @@ public Integer call() throws Exception {
10296
ExtendedExpression extendedExpression = converter.convert(sqlExpressions, createStatements);
10397
printMessage(extendedExpression);
10498
} else { // by default Isthmus image are parsing SQL Query
105-
SqlToSubstrait converter = new SqlToSubstrait(converterProvider());
99+
ConverterProvider provider = converterProvider();
100+
SqlToSubstrait converter = new SqlToSubstrait(provider);
106101
Prepare.CatalogReader catalog =
107102
SubstraitCreateStatementParser.processCreateStatementsToCatalog(
108-
createStatements.toArray(String[]::new));
103+
provider, createStatements);
109104
Plan plan = new PlanProtoConverter().toProto(converter.convert(sql, catalog));
110105
printMessage(plan);
111106
}

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

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ public class ConverterProvider {
5555
/** The collection of Substrait extensions (functions and types) available for conversion. */
5656
protected final SimpleExtension.ExtensionCollection extensions;
5757

58+
/** The casing applied to unquoted SQL identifiers during parsing. */
59+
protected final Casing unquotedCasing;
60+
5861
/** Converter for Substrait scalar functions. */
5962
protected ScalarFunctionConverter scalarFunctionConverter;
6063

@@ -78,6 +81,21 @@ public ConverterProvider() {
7881
this(DefaultExtensionCatalog.DEFAULT_COLLECTION, SubstraitTypeSystem.TYPE_FACTORY);
7982
}
8083

84+
/**
85+
* Creates a ConverterProvider with the specified unquoted identifier casing.
86+
*
87+
* <p>Uses {@link DefaultExtensionCatalog#DEFAULT_COLLECTION} and {@link
88+
* SubstraitTypeSystem#TYPE_FACTORY}.
89+
*
90+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
91+
*/
92+
public ConverterProvider(Casing unquotedCasing) {
93+
this(
94+
DefaultExtensionCatalog.DEFAULT_COLLECTION,
95+
SubstraitTypeSystem.TYPE_FACTORY,
96+
unquotedCasing);
97+
}
98+
8199
/**
82100
* Creates a ConverterProvider with the specified extension collection and default type factory.
83101
*
@@ -95,13 +113,30 @@ public ConverterProvider(SimpleExtension.ExtensionCollection extensions) {
95113
*/
96114
public ConverterProvider(
97115
SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory) {
116+
this(extensions, typeFactory, Casing.TO_UPPER);
117+
}
118+
119+
/**
120+
* Creates a ConverterProvider with the specified extension collection, type factory, and unquoted
121+
* identifier casing.
122+
*
123+
* @param extensions the Substrait extension collection to use
124+
* @param typeFactory the Calcite type factory to use
125+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
126+
*/
127+
public ConverterProvider(
128+
SimpleExtension.ExtensionCollection extensions,
129+
RelDataTypeFactory typeFactory,
130+
Casing unquotedCasing) {
98131
this(
99132
typeFactory,
100133
extensions,
101134
new ScalarFunctionConverter(extensions.scalarFunctions(), typeFactory),
102135
new AggregateFunctionConverter(extensions.aggregateFunctions(), typeFactory),
103136
new WindowFunctionConverter(extensions.windowFunctions(), typeFactory),
104-
TypeConverter.DEFAULT);
137+
TypeConverter.DEFAULT,
138+
createDefaultExecutionBehavior(),
139+
unquotedCasing);
105140
}
106141

107142
/**
@@ -121,7 +156,15 @@ public ConverterProvider(
121156
AggregateFunctionConverter afc,
122157
WindowFunctionConverter wfc,
123158
TypeConverter tc) {
124-
this(typeFactory, extensions, sfc, afc, wfc, tc, createDefaultExecutionBehavior());
159+
this(
160+
typeFactory,
161+
extensions,
162+
sfc,
163+
afc,
164+
wfc,
165+
tc,
166+
createDefaultExecutionBehavior(),
167+
Casing.TO_UPPER);
125168
}
126169

127170
/**
@@ -143,13 +186,39 @@ public ConverterProvider(
143186
WindowFunctionConverter wfc,
144187
TypeConverter tc,
145188
Plan.ExecutionBehavior executionBehavior) {
189+
this(typeFactory, extensions, sfc, afc, wfc, tc, executionBehavior, Casing.TO_UPPER);
190+
}
191+
192+
/**
193+
* Creates a ConverterProvider with full customization including execution behavior and unquoted
194+
* identifier casing.
195+
*
196+
* @param typeFactory the Calcite type factory to use
197+
* @param extensions the Substrait extension collection to use
198+
* @param sfc the scalar function converter to use
199+
* @param afc the aggregate function converter to use
200+
* @param wfc the window function converter to use
201+
* @param tc the type converter to use
202+
* @param executionBehavior the execution behavior to use for plans
203+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
204+
*/
205+
public ConverterProvider(
206+
RelDataTypeFactory typeFactory,
207+
SimpleExtension.ExtensionCollection extensions,
208+
ScalarFunctionConverter sfc,
209+
AggregateFunctionConverter afc,
210+
WindowFunctionConverter wfc,
211+
TypeConverter tc,
212+
Plan.ExecutionBehavior executionBehavior,
213+
Casing unquotedCasing) {
146214
this.typeFactory = typeFactory;
147215
this.extensions = extensions;
148216
this.scalarFunctionConverter = sfc;
149217
this.aggregateFunctionConverter = afc;
150218
this.windowFunctionConverter = wfc;
151219
this.typeConverter = tc;
152220
this.executionBehavior = executionBehavior;
221+
this.unquotedCasing = unquotedCasing;
153222
}
154223

155224
/**
@@ -165,6 +234,15 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
165234

166235
// SQL to Calcite Processing
167236

237+
/**
238+
* Returns the casing applied to unquoted SQL identifiers during parsing.
239+
*
240+
* @return the unquoted identifier casing
241+
*/
242+
public Casing getUnquotedCasing() {
243+
return unquotedCasing;
244+
}
245+
168246
/**
169247
* {@link SqlParser.Config} is a Calcite class which controls SQL parsing behaviour like
170248
* identifier casing.
@@ -173,7 +251,7 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
173251
*/
174252
public SqlParser.Config getSqlParserConfig() {
175253
return SqlParser.Config.DEFAULT
176-
.withUnquotedCasing(Casing.TO_UPPER)
254+
.withUnquotedCasing(unquotedCasing)
177255
.withParserFactory(SqlDdlParserImpl.FACTORY)
178256
.withConformance(SqlConformanceEnum.LENIENT);
179257
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ private Result registerCreateTablesForExtendedExpression(List<String> tables)
202202
if (tables != null) {
203203
for (String tableDef : tables) {
204204
List<SubstraitTable> tList =
205-
SubstraitCreateStatementParser.processCreateStatements(tableDef);
205+
SubstraitCreateStatementParser.processCreateStatements(converterProvider, tableDef);
206206
for (SubstraitTable t : tList) {
207207
rootSchema.add(t.getName(), t);
208208
for (RelDataTypeField field : t.getRowType(factory).getFieldList()) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ public Plan convert(final String sqlStatements, final Prepare.CatalogReader cata
5050
builder.executionBehavior(converterProvider.getExecutionBehavior());
5151

5252
// TODO: consider case in which one sql passes conversion while others don't
53-
SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, operatorTable).stream()
53+
SubstraitSqlToCalcite.convertQueries(
54+
sqlStatements, catalogReader, converterProvider, operatorTable)
55+
.stream()
5456
.map(root -> SubstraitRelVisitor.convert(root, converterProvider))
5557
.forEach(root -> builder.addRoots(root));
5658

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

Lines changed: 109 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.substrait.isthmus.sql;
22

3+
import io.substrait.isthmus.ConverterProvider;
34
import io.substrait.isthmus.SqlConverterBase;
45
import io.substrait.isthmus.SubstraitTypeSystem;
56
import io.substrait.isthmus.Utils;
@@ -16,6 +17,7 @@
1617
import org.apache.calcite.sql.ddl.SqlCreateTable;
1718
import org.apache.calcite.sql.ddl.SqlKeyConstraint;
1819
import org.apache.calcite.sql.parser.SqlParseException;
20+
import org.apache.calcite.sql.parser.SqlParser;
1921
import org.apache.calcite.sql.parser.SqlParserPos;
2022
import org.apache.calcite.sql.validate.SqlValidator;
2123
import org.jspecify.annotations.NonNull;
@@ -50,28 +52,26 @@ public class SubstraitCreateStatementParser {
5052
*/
5153
public static List<SubstraitTable> processCreateStatements(@NonNull final String createStatements)
5254
throws SqlParseException {
53-
final List<SubstraitTable> tableList = new ArrayList<>();
54-
55-
final List<SqlNode> sqlNode = SubstraitSqlStatementParser.parseStatements(createStatements);
56-
for (final SqlNode parsed : sqlNode) {
57-
if (!(parsed instanceof SqlCreateTable)) {
58-
throw fail("Not a valid CREATE TABLE statement.");
59-
}
60-
61-
final SqlCreateTable create = (SqlCreateTable) parsed;
62-
63-
if (create.name.names.size() > 1) {
64-
throw fail("Only simple table names are allowed.", create.name.getParserPosition());
65-
}
66-
67-
if (create.query != null) {
68-
throw fail("CTAS not supported.", create.name.getParserPosition());
69-
}
70-
71-
tableList.add(createSubstraitTable(create.name.names.get(0), create.columnList));
72-
}
55+
return processCreateStatements((SqlParser.Config) null, createStatements);
56+
}
7357

74-
return tableList;
58+
/**
59+
* Parses a SQL string containing only CREATE statements into a list of {@link SubstraitTable}s,
60+
* using the parser settings from the given {@link ConverterProvider}.
61+
*
62+
* <p>This method only supports simple table names without any additional qualifiers. Only used
63+
* with {@link io.substrait.isthmus.SqlExpressionToSubstrait}.
64+
*
65+
* @param converterProvider the converter provider whose parser config controls identifier casing
66+
* and other parser settings
67+
* @param createStatements a SQL string containing only CREATE statements; must not be null
68+
* @return list of {@link SubstraitTable}s generated from the CREATE statements
69+
* @throws SqlParseException if parsing fails or statements are invalid
70+
*/
71+
public static List<SubstraitTable> processCreateStatements(
72+
@NonNull final ConverterProvider converterProvider, @NonNull final String createStatements)
73+
throws SqlParseException {
74+
return processCreateStatements(converterProvider.getSqlParserConfig(), createStatements);
7575
}
7676

7777
/**
@@ -89,6 +89,26 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
8989
return processCreateStatementsToCatalog(createStatements.toArray(new String[0]));
9090
}
9191

92+
/**
93+
* Parses one or more SQL strings containing only CREATE statements into a {@link
94+
* CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
95+
*
96+
* <p>This method expects the use of fully qualified table names in the CREATE statements.
97+
*
98+
* @param converterProvider the converter provider whose parser config controls identifier casing
99+
* and other parser settings
100+
* @param createStatements List of SQL strings containing only CREATE statements, must not be null
101+
* @return a {@link CalciteCatalogReader} generated from the CREATE statements
102+
* @throws SqlParseException if there is an error parsing the SQL statements
103+
*/
104+
public static CalciteCatalogReader processCreateStatementsToCatalog(
105+
@NonNull final ConverterProvider converterProvider,
106+
@NonNull final List<String> createStatements)
107+
throws SqlParseException {
108+
return processCreateStatementsToCatalog(
109+
converterProvider, createStatements.toArray(new String[0]));
110+
}
111+
92112
/**
93113
* Parses one or more SQL strings containing only CREATE statements into a {@link
94114
* CalciteCatalogReader}
@@ -101,7 +121,32 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
101121
*/
102122
public static CalciteCatalogReader processCreateStatementsToCatalog(
103123
@NonNull final String... createStatements) throws SqlParseException {
104-
final CalciteSchema rootSchema = processCreateStatementsToSchema(createStatements);
124+
final CalciteSchema rootSchema = processCreateStatementsToSchema(null, createStatements);
125+
final List<String> defaultSchema = Collections.emptyList();
126+
return new CalciteCatalogReader(
127+
rootSchema,
128+
defaultSchema,
129+
SubstraitTypeSystem.TYPE_FACTORY,
130+
SqlConverterBase.CONNECTION_CONFIG);
131+
}
132+
133+
/**
134+
* Parses one or more SQL strings containing only CREATE statements into a {@link
135+
* CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
136+
*
137+
* <p>This method expects the use of fully qualified table names in the CREATE statements.
138+
*
139+
* @param converterProvider the converter provider whose parser config controls identifier casing
140+
* and other parser settings
141+
* @param createStatements a SQL string containing only CREATE statements, must not be null
142+
* @return a {@link CalciteCatalogReader} generated from the CREATE statements
143+
* @throws SqlParseException if parsing fails or statements are invalid
144+
*/
145+
public static CalciteCatalogReader processCreateStatementsToCatalog(
146+
@NonNull final ConverterProvider converterProvider, @NonNull final String... createStatements)
147+
throws SqlParseException {
148+
final CalciteSchema rootSchema =
149+
processCreateStatementsToSchema(converterProvider.getSqlParserConfig(), createStatements);
105150
final List<String> defaultSchema = Collections.emptyList();
106151
return new CalciteCatalogReader(
107152
rootSchema,
@@ -133,19 +178,53 @@ private static SqlParseException fail(@Nullable final String message) {
133178
}
134179

135180
/**
136-
* Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}.
137-
*
138-
* @param createStatements one or more SQL strings containing only CREATE statements; must not be
139-
* null
140-
* @return a {@link CalciteSchema} generated from the CREATE statements
141-
* @throws SqlParseException if parsing fails or statements are invalid
181+
* Parses a SQL string containing only CREATE statements into a list of {@link SubstraitTable}s
182+
* using the given parser config (may be {@code null} for the default).
183+
*/
184+
private static List<SubstraitTable> processCreateStatements(
185+
SqlParser.Config parserConfig, @NonNull final String createStatements)
186+
throws SqlParseException {
187+
final List<SubstraitTable> tableList = new ArrayList<>();
188+
189+
final List<SqlNode> sqlNode =
190+
parserConfig == null
191+
? SubstraitSqlStatementParser.parseStatements(createStatements)
192+
: SubstraitSqlStatementParser.parseStatements(createStatements, parserConfig);
193+
for (final SqlNode parsed : sqlNode) {
194+
if (!(parsed instanceof SqlCreateTable)) {
195+
throw fail("Not a valid CREATE TABLE statement.");
196+
}
197+
198+
final SqlCreateTable create = (SqlCreateTable) parsed;
199+
200+
if (create.name.names.size() > 1) {
201+
throw fail("Only simple table names are allowed.", create.name.getParserPosition());
202+
}
203+
204+
if (create.query != null) {
205+
throw fail("CTAS not supported.", create.name.getParserPosition());
206+
}
207+
208+
tableList.add(createSubstraitTable(create.name.names.get(0), create.columnList));
209+
}
210+
211+
return tableList;
212+
}
213+
214+
/**
215+
* Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}
216+
* using the given parser config (may be {@code null} for the default).
142217
*/
143218
private static CalciteSchema processCreateStatementsToSchema(
144-
@NonNull final String... createStatements) throws SqlParseException {
219+
SqlParser.Config parserConfig, @NonNull final String... createStatements)
220+
throws SqlParseException {
145221
final CalciteSchema rootSchema = CalciteSchema.createRootSchema(false);
146222

147223
for (final String statement : createStatements) {
148-
final List<SqlNode> sqlNode = SubstraitSqlStatementParser.parseStatements(statement);
224+
final List<SqlNode> sqlNode =
225+
parserConfig == null
226+
? SubstraitSqlStatementParser.parseStatements(statement)
227+
: SubstraitSqlStatementParser.parseStatements(statement, parserConfig);
149228
for (final SqlNode parsed : sqlNode) {
150229
if (!(parsed instanceof SqlCreateTable)) {
151230
throw fail("Not a valid CREATE TABLE statement.");

0 commit comments

Comments
 (0)