Skip to content

Commit 05df340

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 05df340

9 files changed

Lines changed: 359 additions & 71 deletions

File tree

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

Lines changed: 4 additions & 13 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;
@@ -60,15 +59,6 @@ enum OutputFormat {
6059
description = "Calcite's casing policy for unquoted identifiers: ${COMPLETION-CANDIDATES}")
6160
private Casing unquotedCasing = Casing.TO_UPPER;
6261

63-
private ConverterProvider converterProvider() {
64-
return new ConverterProvider() {
65-
@Override
66-
public SqlParser.Config getSqlParserConfig() {
67-
return super.getSqlParserConfig().withUnquotedCasing(unquotedCasing);
68-
}
69-
};
70-
}
71-
7262
/**
7363
* Standard Java Main method invoked by the isthmus CLI command.
7464
*
@@ -96,16 +86,17 @@ public static void main(String... args) {
9686

9787
@Override
9888
public Integer call() throws Exception {
89+
ConverterProvider provider = new ConverterProvider(unquotedCasing);
9990
// Isthmus image is parsing SQL Expression if that argument is defined
10091
if (sqlExpressions != null) {
101-
SqlExpressionToSubstrait converter = new SqlExpressionToSubstrait(converterProvider());
92+
SqlExpressionToSubstrait converter = new SqlExpressionToSubstrait(provider);
10293
ExtendedExpression extendedExpression = converter.convert(sqlExpressions, createStatements);
10394
printMessage(extendedExpression);
10495
} else { // by default Isthmus image are parsing SQL Query
105-
SqlToSubstrait converter = new SqlToSubstrait(converterProvider());
96+
SqlToSubstrait converter = new SqlToSubstrait(provider);
10697
Prepare.CatalogReader catalog =
10798
SubstraitCreateStatementParser.processCreateStatementsToCatalog(
108-
createStatements.toArray(String[]::new));
99+
provider, createStatements);
109100
Plan plan = new PlanProtoConverter().toProto(converter.convert(sql, catalog));
110101
printMessage(plan);
111102
}

‎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: 94 additions & 29 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;
@@ -50,28 +51,7 @@ public class SubstraitCreateStatementParser {
5051
*/
5152
public static List<SubstraitTable> processCreateStatements(@NonNull final String createStatements)
5253
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-
}
73-
74-
return tableList;
54+
return processCreateStatements(new ConverterProvider(), createStatements);
7555
}
7656

7757
/**
@@ -89,6 +69,26 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
8969
return processCreateStatementsToCatalog(createStatements.toArray(new String[0]));
9070
}
9171

72+
/**
73+
* Parses one or more SQL strings containing only CREATE statements into a {@link
74+
* CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
75+
*
76+
* <p>This method expects the use of fully qualified table names in the CREATE statements.
77+
*
78+
* @param converterProvider the converter provider whose parser config controls identifier casing
79+
* and other parser settings
80+
* @param createStatements List of SQL strings containing only CREATE statements, must not be null
81+
* @return a {@link CalciteCatalogReader} generated from the CREATE statements
82+
* @throws SqlParseException if there is an error parsing the SQL statements
83+
*/
84+
public static CalciteCatalogReader processCreateStatementsToCatalog(
85+
@NonNull final ConverterProvider converterProvider,
86+
@NonNull final List<String> createStatements)
87+
throws SqlParseException {
88+
return processCreateStatementsToCatalog(
89+
converterProvider, createStatements.toArray(new String[0]));
90+
}
91+
9292
/**
9393
* Parses one or more SQL strings containing only CREATE statements into a {@link
9494
* CalciteCatalogReader}
@@ -101,7 +101,33 @@ public static CalciteCatalogReader processCreateStatementsToCatalog(
101101
*/
102102
public static CalciteCatalogReader processCreateStatementsToCatalog(
103103
@NonNull final String... createStatements) throws SqlParseException {
104-
final CalciteSchema rootSchema = processCreateStatementsToSchema(createStatements);
104+
final CalciteSchema rootSchema =
105+
processCreateStatementsToSchema(new ConverterProvider(), createStatements);
106+
final List<String> defaultSchema = Collections.emptyList();
107+
return new CalciteCatalogReader(
108+
rootSchema,
109+
defaultSchema,
110+
SubstraitTypeSystem.TYPE_FACTORY,
111+
SqlConverterBase.CONNECTION_CONFIG);
112+
}
113+
114+
/**
115+
* Parses one or more SQL strings containing only CREATE statements into a {@link
116+
* CalciteCatalogReader}, using the parser settings from the given {@link ConverterProvider}.
117+
*
118+
* <p>This method expects the use of fully qualified table names in the CREATE statements.
119+
*
120+
* @param converterProvider the converter provider whose parser config controls identifier casing
121+
* and other parser settings
122+
* @param createStatements a SQL string containing only CREATE statements, must not be null
123+
* @return a {@link CalciteCatalogReader} generated from the CREATE statements
124+
* @throws SqlParseException if parsing fails or statements are invalid
125+
*/
126+
public static CalciteCatalogReader processCreateStatementsToCatalog(
127+
@NonNull final ConverterProvider converterProvider, @NonNull final String... createStatements)
128+
throws SqlParseException {
129+
final CalciteSchema rootSchema =
130+
processCreateStatementsToSchema(converterProvider, createStatements);
105131
final List<String> defaultSchema = Collections.emptyList();
106132
return new CalciteCatalogReader(
107133
rootSchema,
@@ -133,19 +159,58 @@ private static SqlParseException fail(@Nullable final String message) {
133159
}
134160

135161
/**
136-
* Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}.
162+
* Parses a SQL string containing only CREATE statements into a list of {@link SubstraitTable}s,
163+
* using the parser settings from the given {@link ConverterProvider}.
164+
*
165+
* <p>This method only supports simple table names without any additional qualifiers. Only used
166+
* with {@link io.substrait.isthmus.SqlExpressionToSubstrait}.
137167
*
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
168+
* @param converterProvider the converter provider whose parser config controls identifier casing
169+
* and other parser settings
170+
* @param createStatements a SQL string containing only CREATE statements; must not be null
171+
* @return list of {@link SubstraitTable}s generated from the CREATE statements
141172
* @throws SqlParseException if parsing fails or statements are invalid
142173
*/
174+
public static List<SubstraitTable> processCreateStatements(
175+
@NonNull final ConverterProvider converterProvider, @NonNull final String createStatements)
176+
throws SqlParseException {
177+
final List<SubstraitTable> tableList = new ArrayList<>();
178+
179+
final List<SqlNode> sqlNode =
180+
SubstraitSqlStatementParser.parseStatements(createStatements, converterProvider);
181+
for (final SqlNode parsed : sqlNode) {
182+
if (!(parsed instanceof SqlCreateTable)) {
183+
throw fail("Not a valid CREATE TABLE statement.");
184+
}
185+
186+
final SqlCreateTable create = (SqlCreateTable) parsed;
187+
188+
if (create.name.names.size() > 1) {
189+
throw fail("Only simple table names are allowed.", create.name.getParserPosition());
190+
}
191+
192+
if (create.query != null) {
193+
throw fail("CTAS not supported.", create.name.getParserPosition());
194+
}
195+
196+
tableList.add(createSubstraitTable(create.name.names.get(0), create.columnList));
197+
}
198+
199+
return tableList;
200+
}
201+
202+
/**
203+
* Parses one or more SQL strings containing only CREATE statements into a {@link CalciteSchema}
204+
* using the given provider's parser config.
205+
*/
143206
private static CalciteSchema processCreateStatementsToSchema(
144-
@NonNull final String... createStatements) throws SqlParseException {
207+
@NonNull final ConverterProvider converterProvider, @NonNull final String... createStatements)
208+
throws SqlParseException {
145209
final CalciteSchema rootSchema = CalciteSchema.createRootSchema(false);
146210

147211
for (final String statement : createStatements) {
148-
final List<SqlNode> sqlNode = SubstraitSqlStatementParser.parseStatements(statement);
212+
final List<SqlNode> sqlNode =
213+
SubstraitSqlStatementParser.parseStatements(statement, converterProvider);
149214
for (final SqlNode parsed : sqlNode) {
150215
if (!(parsed instanceof SqlCreateTable)) {
151216
throw fail("Not a valid CREATE TABLE statement.");

0 commit comments

Comments
 (0)