Skip to content

Commit 2467c50

Browse files
committed
feat(isthmus): make unquoted identifier casing configurable in ConverterProvider
Add `withUnquotedCasing(Casing)` and `getUnquotedCasing()` to `ConverterProvider` so callers can control how unquoted SQL identifiers are cased. The default remains `Casing.TO_UPPER`. The setting is now propagated consistently through the full pipeline: - `getSqlParserConfig()` uses the stored casing value - `SubstraitSqlToCalcite.convertQueries` gains a `ConverterProvider` overload so query parsing respects the provider config - `SubstraitCreateStatementParser.processCreateStatements/ToCatalog` gain `ConverterProvider` overloads so CREATE TABLE parsing uses the same casing, keeping catalog names and plan names in sync - `SqlExpressionToSubstrait` and `IsthmusEntryPoint` updated to pass the provider rather than extracting `SqlParser.Config` themselves; the CLI anonymous subclass workaround is removed Closes #<issue>
1 parent fbf2930 commit 2467c50

7 files changed

Lines changed: 260 additions & 42 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().withUnquotedCasing(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: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ public class ConverterProvider {
5555
/** The collection of Substrait extensions (functions and types) available for conversion. */
5656
protected final SimpleExtension.ExtensionCollection extensions;
5757

58+
/**
59+
* The casing applied to unquoted SQL identifiers during parsing. Defaults to {@link
60+
* Casing#TO_UPPER}.
61+
*/
62+
protected Casing unquotedCasing = Casing.TO_UPPER;
63+
5864
/** Converter for Substrait scalar functions. */
5965
protected ScalarFunctionConverter scalarFunctionConverter;
6066

@@ -165,6 +171,30 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
165171

166172
// SQL to Calcite Processing
167173

174+
/**
175+
* Returns the casing applied to unquoted SQL identifiers during parsing.
176+
*
177+
* @return the unquoted identifier casing
178+
*/
179+
public Casing getUnquotedCasing() {
180+
return unquotedCasing;
181+
}
182+
183+
/**
184+
* Sets the casing applied to unquoted SQL identifiers during parsing and returns this instance
185+
* for method chaining.
186+
*
187+
* <p>The default is {@link Casing#TO_UPPER}. Use {@link Casing#UNCHANGED} to preserve the
188+
* original casing of unquoted identifiers in SQL statements.
189+
*
190+
* @param unquotedCasing the casing to apply to unquoted identifiers
191+
* @return this {@code ConverterProvider} instance
192+
*/
193+
public ConverterProvider withUnquotedCasing(Casing unquotedCasing) {
194+
this.unquotedCasing = unquotedCasing;
195+
return this;
196+
}
197+
168198
/**
169199
* {@link SqlParser.Config} is a Calcite class which controls SQL parsing behaviour like
170200
* identifier casing.
@@ -173,7 +203,7 @@ private static Plan.ExecutionBehavior createDefaultExecutionBehavior() {
173203
*/
174204
public SqlParser.Config getSqlParserConfig() {
175205
return SqlParser.Config.DEFAULT
176-
.withUnquotedCasing(Casing.TO_UPPER)
206+
.withUnquotedCasing(unquotedCasing)
177207
.withParserFactory(SqlDdlParserImpl.FACTORY)
178208
.withConformance(SqlConformanceEnum.LENIENT);
179209
}

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.");

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

Lines changed: 29 additions & 0 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.SubstraitTypeSystem;
45
import io.substrait.isthmus.calcite.rel.DdlSqlToRelConverter;
56
import java.util.List;
@@ -109,6 +110,34 @@ public static List<RelRoot> convertQueries(
109110
return convertQueries(sqlStatements, catalogReader, validator, createDefaultRelOptCluster());
110111
}
111112

113+
/**
114+
* Converts one or more SQL statements to a List of {@link RelRoot}, with one {@link RelRoot} per
115+
* statement, using the parser configuration from the given {@link ConverterProvider}.
116+
*
117+
* @param sqlStatements a string containing one or more SQL statements
118+
* @param catalogReader the {@link Prepare.CatalogReader} for finding tables/views referenced in
119+
* the SQL statements
120+
* @param converterProvider the converter provider whose parser config controls identifier casing
121+
* and other parser settings
122+
* @param operatorTable the {@link SqlOperatorTable} for controlling valid operators
123+
* @return a list of {@link RelRoot}s corresponding to the given SQL statements
124+
* @throws SqlParseException if there is an error while parsing the SQL statements
125+
*/
126+
public static List<RelRoot> convertQueries(
127+
String sqlStatements,
128+
Prepare.CatalogReader catalogReader,
129+
ConverterProvider converterProvider,
130+
SqlOperatorTable operatorTable)
131+
throws SqlParseException {
132+
SqlValidator validator = new SubstraitSqlValidator(catalogReader, operatorTable);
133+
return convertQueries(
134+
sqlStatements,
135+
catalogReader,
136+
validator,
137+
createDefaultRelOptCluster(),
138+
converterProvider.getSqlParserConfig());
139+
}
140+
112141
/**
113142
* Converts one or more SQL statements to a List of {@link RelRoot}, with one {@link RelRoot} per
114143
* statement.

0 commit comments

Comments
 (0)