Skip to content

Commit 2285cd1

Browse files
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; the legacy convert(sql, catalog, SqlDialect) overload is deprecated and now delegates to it (the SqlDialect argument is ignored — casing is controlled by the ConverterProvider) - 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 Examples: - FromSql: replaced the deprecated convert(sql, catalog, SqlDialect) call with a shared ConverterProvider(Casing.UNCHANGED) used for both schema and query parsing, preserving the lower-case identifiers as written
1 parent a29fbb1 commit 2285cd1

12 files changed

Lines changed: 356 additions & 133 deletions

File tree

examples/isthmus-api/src/main/java/io/substrait/examples/FromSql.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.substrait.examples;
22

33
import io.substrait.examples.IsthmusAppExamples.Action;
4+
import io.substrait.isthmus.ConverterProvider;
45
import io.substrait.isthmus.SqlToSubstrait;
56
import io.substrait.isthmus.sql.SubstraitCreateStatementParser;
67
import io.substrait.plan.Plan;
@@ -10,8 +11,8 @@
1011
import java.nio.file.Path;
1112
import java.nio.file.Paths;
1213
import java.util.List;
14+
import org.apache.calcite.avatica.util.Casing;
1315
import org.apache.calcite.prepare.CalciteCatalogReader;
14-
import org.apache.calcite.sql.SqlDialect;
1516
import org.apache.calcite.sql.parser.SqlParseException;
1617

1718
/**
@@ -22,7 +23,7 @@
2223
* <p>1. Create a fully typed schema for the inputs. Within a SQL context this represents the CREATE
2324
* TABLE commands, which need to be converted to a Calcite Schema.
2425
*
25-
* <p>2. Parse the SQL query to convert (in the source SQL dialect).
26+
* <p>2. Parse the SQL query to convert.
2627
*
2728
* <p>3. Convert the SQL query to Calcite Relations.
2829
*
@@ -49,22 +50,26 @@ public void run(final String[] args) {
4950
"test_result" varchar(15),"test_mileage" int, "postcode_area" varchar(15));
5051
""");
5152

53+
// The unquoted identifier casing applied while parsing is configurable via a
54+
// ConverterProvider. The same provider is used for both the schema and the query so that
55+
// identifier casing stays consistent end-to-end. Casing.UNCHANGED preserves identifiers as
56+
// written, matching the lower-case names used in the CREATE TABLE statements above.
57+
final ConverterProvider converterProvider = new ConverterProvider(Casing.UNCHANGED);
58+
5259
final CalciteCatalogReader catalogReader =
53-
SubstraitCreateStatementParser.processCreateStatementsToCatalog(createSqlStatements);
60+
SubstraitCreateStatementParser.processCreateStatementsToCatalog(
61+
converterProvider, createSqlStatements);
5462

55-
// Query that needs to be converted; again this could be in a variety of SQL
56-
// dialects
63+
// Query that needs to be converted
5764
final String sqlQuery =
5865
"""
5966
SELECT vehicles.colour, count(*) as colourcount FROM vehicles INNER JOIN tests
6067
ON vehicles.vehicle_id=tests.vehicle_id WHERE tests.test_result = 'P'
6168
GROUP BY vehicles.colour ORDER BY count(*)
6269
""";
63-
final SqlToSubstrait sqlToSubstrait = new SqlToSubstrait();
6470

65-
// choose DuckDB as an example dialect
66-
final SqlDialect dialect = SqlDialect.DatabaseProduct.DUCKDB.getDialect();
67-
final Plan substraitPlan = sqlToSubstrait.convert(sqlQuery, catalogReader, dialect);
71+
final SqlToSubstrait sqlToSubstrait = new SqlToSubstrait(converterProvider);
72+
final Plan substraitPlan = sqlToSubstrait.convert(sqlQuery, catalogReader);
6873

6974
// Create the proto plan to display to stdout - as it has a better format
7075
final PlanProtoConverter planToProto = new PlanProtoConverter();

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: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,24 @@
4949
*/
5050
public class ConverterProvider {
5151

52+
/**
53+
* A shared default {@link ConverterProvider} instance using all system defaults. Equivalent to
54+
* {@code new ConverterProvider()} but avoids redundant construction at every call site.
55+
*
56+
* <p>This instance is safe to share because {@link ConverterProvider} is effectively immutable
57+
* after construction — all fields are set only in constructors.
58+
*/
59+
public static final ConverterProvider DEFAULT = new ConverterProvider();
60+
5261
/** The Calcite type factory used for creating and managing data types. */
5362
protected RelDataTypeFactory typeFactory;
5463

5564
/** The collection of Substrait extensions (functions and types) available for conversion. */
5665
protected final SimpleExtension.ExtensionCollection extensions;
5766

67+
/** The casing applied to unquoted SQL identifiers during parsing. */
68+
protected final Casing unquotedCasing;
69+
5870
/** Converter for Substrait scalar functions. */
5971
protected ScalarFunctionConverter scalarFunctionConverter;
6072

@@ -78,6 +90,21 @@ public ConverterProvider() {
7890
this(DefaultExtensionCatalog.DEFAULT_COLLECTION, SubstraitTypeSystem.TYPE_FACTORY);
7991
}
8092

93+
/**
94+
* Creates a ConverterProvider with the specified unquoted identifier casing.
95+
*
96+
* <p>Uses {@link DefaultExtensionCatalog#DEFAULT_COLLECTION} and {@link
97+
* SubstraitTypeSystem#TYPE_FACTORY}.
98+
*
99+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
100+
*/
101+
public ConverterProvider(Casing unquotedCasing) {
102+
this(
103+
DefaultExtensionCatalog.DEFAULT_COLLECTION,
104+
SubstraitTypeSystem.TYPE_FACTORY,
105+
unquotedCasing);
106+
}
107+
81108
/**
82109
* Creates a ConverterProvider with the specified extension collection and default type factory.
83110
*
@@ -95,13 +122,30 @@ public ConverterProvider(SimpleExtension.ExtensionCollection extensions) {
95122
*/
96123
public ConverterProvider(
97124
SimpleExtension.ExtensionCollection extensions, RelDataTypeFactory typeFactory) {
125+
this(extensions, typeFactory, Casing.TO_UPPER);
126+
}
127+
128+
/**
129+
* Creates a ConverterProvider with the specified extension collection, type factory, and unquoted
130+
* identifier casing.
131+
*
132+
* @param extensions the Substrait extension collection to use
133+
* @param typeFactory the Calcite type factory to use
134+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
135+
*/
136+
public ConverterProvider(
137+
SimpleExtension.ExtensionCollection extensions,
138+
RelDataTypeFactory typeFactory,
139+
Casing unquotedCasing) {
98140
this(
99141
typeFactory,
100142
extensions,
101143
new ScalarFunctionConverter(extensions.scalarFunctions(), typeFactory),
102144
new AggregateFunctionConverter(extensions.aggregateFunctions(), typeFactory),
103145
new WindowFunctionConverter(extensions.windowFunctions(), typeFactory),
104-
TypeConverter.DEFAULT);
146+
TypeConverter.DEFAULT,
147+
createDefaultExecutionBehavior(),
148+
unquotedCasing);
105149
}
106150

107151
/**
@@ -121,7 +165,15 @@ public ConverterProvider(
121165
AggregateFunctionConverter afc,
122166
WindowFunctionConverter wfc,
123167
TypeConverter tc) {
124-
this(typeFactory, extensions, sfc, afc, wfc, tc, createDefaultExecutionBehavior());
168+
this(
169+
typeFactory,
170+
extensions,
171+
sfc,
172+
afc,
173+
wfc,
174+
tc,
175+
createDefaultExecutionBehavior(),
176+
Casing.TO_UPPER);
125177
}
126178

127179
/**
@@ -143,13 +195,39 @@ public ConverterProvider(
143195
WindowFunctionConverter wfc,
144196
TypeConverter tc,
145197
Plan.ExecutionBehavior executionBehavior) {
198+
this(typeFactory, extensions, sfc, afc, wfc, tc, executionBehavior, Casing.TO_UPPER);
199+
}
200+
201+
/**
202+
* Creates a ConverterProvider with full customization including execution behavior and unquoted
203+
* identifier casing.
204+
*
205+
* @param typeFactory the Calcite type factory to use
206+
* @param extensions the Substrait extension collection to use
207+
* @param sfc the scalar function converter to use
208+
* @param afc the aggregate function converter to use
209+
* @param wfc the window function converter to use
210+
* @param tc the type converter to use
211+
* @param executionBehavior the execution behavior to use for plans
212+
* @param unquotedCasing the casing to apply to unquoted SQL identifiers during parsing
213+
*/
214+
public ConverterProvider(
215+
RelDataTypeFactory typeFactory,
216+
SimpleExtension.ExtensionCollection extensions,
217+
ScalarFunctionConverter sfc,
218+
AggregateFunctionConverter afc,
219+
WindowFunctionConverter wfc,
220+
TypeConverter tc,
221+
Plan.ExecutionBehavior executionBehavior,
222+
Casing unquotedCasing) {
146223
this.typeFactory = typeFactory;
147224
this.extensions = extensions;
148225
this.scalarFunctionConverter = sfc;
149226
this.aggregateFunctionConverter = afc;
150227
this.windowFunctionConverter = wfc;
151228
this.typeConverter = tc;
152229
this.executionBehavior = executionBehavior;
230+
this.unquotedCasing = unquotedCasing;
153231
}
154232

155233
/**
@@ -165,6 +243,15 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
165243

166244
// SQL to Calcite Processing
167245

246+
/**
247+
* Returns the casing applied to unquoted SQL identifiers during parsing.
248+
*
249+
* @return the unquoted identifier casing
250+
*/
251+
public Casing getUnquotedCasing() {
252+
return unquotedCasing;
253+
}
254+
168255
/**
169256
* {@link SqlParser.Config} is a Calcite class which controls SQL parsing behaviour like
170257
* identifier casing.
@@ -173,7 +260,7 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
173260
*/
174261
public SqlParser.Config getSqlParserConfig() {
175262
return SqlParser.Config.DEFAULT
176-
.withUnquotedCasing(Casing.TO_UPPER)
263+
.withUnquotedCasing(unquotedCasing)
177264
.withParserFactory(SqlDdlParserImpl.FACTORY)
178265
.withConformance(SqlConformanceEnum.LENIENT);
179266
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public class SqlExpressionToSubstrait extends SqlConverterBase {
4040

4141
/** Creates a converter with default configuration. */
4242
public SqlExpressionToSubstrait() {
43-
this(new ConverterProvider());
43+
this(ConverterProvider.DEFAULT);
4444
}
4545

4646
/**
@@ -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: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55
import io.substrait.plan.Plan;
66
import io.substrait.plan.Plan.Version;
77
import org.apache.calcite.prepare.Prepare;
8-
import org.apache.calcite.server.ServerDdlExecutor;
98
import org.apache.calcite.sql.SqlDialect;
109
import org.apache.calcite.sql.SqlOperatorTable;
1110
import org.apache.calcite.sql.parser.SqlParseException;
12-
import org.apache.calcite.sql.parser.SqlParser;
1311

1412
/**
1513
* Take a SQL statement and a set of table definitions and return a substrait plan.
@@ -21,7 +19,7 @@ public class SqlToSubstrait extends SqlConverterBase {
2119

2220
/** Creates a SQL-to-Substrait converter using the default configuration. */
2321
public SqlToSubstrait() {
24-
this(new ConverterProvider());
22+
this(ConverterProvider.DEFAULT);
2523
}
2624

2725
/**
@@ -50,7 +48,9 @@ public Plan convert(final String sqlStatements, final Prepare.CatalogReader cata
5048
builder.executionBehavior(converterProvider.getExecutionBehavior());
5149

5250
// TODO: consider case in which one sql passes conversion while others don't
53-
SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, operatorTable).stream()
51+
SubstraitSqlToCalcite.convertQueries(
52+
sqlStatements, catalogReader, converterProvider, operatorTable)
53+
.stream()
5454
.map(root -> SubstraitRelVisitor.convert(root, converterProvider))
5555
.forEach(root -> builder.addRoots(root));
5656

@@ -60,31 +60,29 @@ public Plan convert(final String sqlStatements, final Prepare.CatalogReader cata
6060
/**
6161
* Converts one or more SQL statements into a Substrait {@link Plan}.
6262
*
63+
* <p>The {@code sqlDialect} parameter was previously used to influence identifier casing during
64+
* parsing. This is now configurable directly via {@link
65+
* ConverterProvider#ConverterProvider(org.apache.calcite.avatica.util.Casing)}, or for fully
66+
* custom parser behaviour, by subclassing {@link ConverterProvider} and overriding {@link
67+
* ConverterProvider#getSqlParserConfig()}.
68+
*
6369
* @param sqlStatements a string containing one more SQL statements
6470
* @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in
6571
* the SQL statements
6672
* @param sqlDialect The sql dialect to use for parsing.
6773
* @return the Substrait {@link Plan}
6874
* @throws SqlParseException if there is an error while parsing the SQL statements
75+
* @deprecated Prefer constructing {@link SqlToSubstrait} with a {@link ConverterProvider}
76+
* configured for the desired casing and calling {@link #convert(String,
77+
* Prepare.CatalogReader)}. For fully custom parser behaviour, subclass {@link
78+
* ConverterProvider} and override {@link ConverterProvider#getSqlParserConfig()}.
6979
*/
80+
@Deprecated
7081
public Plan convert(
7182
final String sqlStatements,
7283
final Prepare.CatalogReader catalogReader,
7384
final SqlDialect sqlDialect)
7485
throws SqlParseException {
75-
Builder builder = io.substrait.plan.Plan.builder();
76-
builder.version(Version.builder().from(Version.DEFAULT_VERSION).producer("isthmus").build());
77-
builder.executionBehavior(converterProvider.getExecutionBehavior());
78-
79-
final SqlParser.Config sqlParserConfig =
80-
sqlDialect.configureParser(
81-
SqlParser.config().withParserFactory(ServerDdlExecutor.PARSER_FACTORY));
82-
83-
// TODO: consider case in which one sql passes conversion while others don't
84-
SubstraitSqlToCalcite.convertQueries(sqlStatements, catalogReader, sqlParserConfig).stream()
85-
.map(root -> SubstraitRelVisitor.convert(root, converterProvider))
86-
.forEach(root -> builder.addRoots(root));
87-
88-
return builder.build();
86+
return convert(sqlStatements, catalogReader);
8987
}
9088
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public class SubstraitToSql extends SqlConverterBase {
2323

2424
/** Creates a SubstraitToSql converter with default configuration and extensions. */
2525
public SubstraitToSql() {
26-
this(new ConverterProvider());
26+
this(ConverterProvider.DEFAULT);
2727
}
2828

2929
/**

0 commit comments

Comments
 (0)