feat(isthmus)!: make unquoted identifier casing configurable in ConverterProvider

[nielspardon]

Summary

Adds 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.

Breaking change

The SqlParser.Config-based parsing entry points (public in v0.94.0) have been removed in favour of ConverterProvider overloads:

• SubstraitSqlStatementParser.parseStatements(String, SqlParser.Config)
• SubstraitSqlToCalcite.convertQueries(String, CatalogReader, SqlParser.Config)
• SubstraitSqlToCalcite.convertQueries(String, CatalogReader, SqlValidator, RelOptCluster, SqlParser.Config)

Callers should pass a ConverterProvider (constructed with the desired Casing, or subclassed with an overridden getSqlParserConfig() for fully custom parser settings) instead of a SqlParser.Config.

Changes

ConverterProvider

• unquotedCasing is a new final field, consistent with executionBehavior
• getUnquotedCasing() — getter
• getSqlParserConfig() reads unquotedCasing instead of hard-coding Casing.TO_UPPER
• New constructors: ConverterProvider(Casing) and ConverterProvider(extensions, typeFactory, Casing) for the common cases; the existing 7-arg all-components constructor gains Casing as an 8th parameter. All narrower constructors default to Casing.TO_UPPER.
• ConverterProvider.DEFAULT — a shared constant for the default (all-system-defaults) provider, used at every call site that previously wrote new ConverterProvider().

Propagation through the pipeline

The casing setting is applied consistently across both CREATE TABLE parsing and query parsing, so that the table name stored in a NamedScan matches the configured casing end-to-end.

Class	Change
SubstraitSqlStatementParser	parseStatements(String, ConverterProvider) now owns the SqlParser instantiation directly; the SqlParser.Config overload is removed — callers needing fully custom config should subclass ConverterProvider and override getSqlParserConfig()
SubstraitSqlToCalcite	New convertQueries(sql, catalog, ConverterProvider) and convertQueries(sql, catalog, ConverterProvider, operatorTable) overloads; all internal overloads now route through ConverterProvider; the SqlParser.Config overloads are removed
SubstraitCreateStatementParser	New processCreateStatements(ConverterProvider, sql) and processCreateStatementsToCatalog(ConverterProvider, ...) overloads
SqlToSubstrait	convert(sql, catalog) now uses the ConverterProvider path; convert(sql, catalog, SqlDialect) is @Deprecated — the SqlDialect argument is ignored and it simply delegates to convert(sql, catalog)
SqlExpressionToSubstrait	Uses processCreateStatements(converterProvider, tableDef)
SubstraitToSql	No-arg constructor uses ConverterProvider.DEFAULT
IsthmusEntryPoint	Uses new ConverterProvider(unquotedCasing); anonymous ConverterProvider subclass removed
FromSql (example)	Replaces the deprecated convert(sql, catalog, SqlDialect) call with a single ConverterProvider(Casing.UNCHANGED) shared across both the schema build and the query conversion, preserving the lower-case identifiers as written

Test

UnquotedCasingTest verifies:

• The default casing is TO_UPPER and is reflected in getSqlParserConfig()
• new ConverterProvider(Casing) sets the casing correctly for all three Casing values
• End-to-end: with TO_UPPER a plan built from CREATE TABLE employees … / SELECT … FROM employees produces a NamedScan with name EMPLOYEES; with UNCHANGED it produces employees

Existing tests DdlToSubstraitConversionTest and DdlToSubstraitConversionWithOptimizationTest are updated to use the ConverterProvider API instead of the removed SqlParser.Config overloads.

Notes

For consumers who need a fully custom parser configuration beyond what ConverterProvider exposes (e.g. a different parser factory), the supported extension point is subclassing ConverterProvider and overriding getSqlParserConfig(). The SqlParser.Config overloads on SubstraitSqlStatementParser and SubstraitSqlToCalcite have been removed since they are entirely superseded by this.

[vbarua]

I don't think we should have an unquotedCasing option like this on the ConverterProvider.

SqlParser.Config has a ton of options on it. Historically, the issue that we ran into was that we added flags to toggle bits and pieces as we needed.

The reason this is structured like this to return a full SqlParser.Config is to allow users to extend the ConverterProvider to return their system specific parsing config.

[vbarua]

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.

Left a code comment to this effect, but this actually not a feature I think we should support. SQL parsing is very much a Calcite concern, and for users that need to customize it I would push them to handle their parsing purely in Calcite and then hand the Calcite plans to the Isthmus converters.

Is there a specific usecase that you want this for?

[nielspardon]

TL;DR: isthmus hard-codes Casing.TO_UPPER, so plans it generates carry upper-cased identifiers (ORDERS, O_ORDERKEY) that don't match engines which fold to lower-case (Presto/Trino, Spark, DuckDB, Postgres). The isthmus CLI already exposes --unquotedcasing for exactly this — but the only way it can implement that today is an anonymous ConverterProvider subclass, which every other consumer then has to duplicate. This PR makes casing first-class so isthmus-generated plans are portable to non-upper-casing engines. Casing is worth a dedicated knob (vs. the general getSqlParserConfig() override) because, unlike most SqlParser.Config options, it changes the output — the identifier names written into the plan — not just how SQL text is parsed. Happy to keep the SqlParser.Config overloads so it's non-breaking.

---

Hi Victor — thanks, both points are fair. Here's the concrete use case that drove this, and why casing specifically feels different from the general "one more SqlParser.Config toggle" concern.

The specific use case: Substrait plans generated by isthmus don't round-trip against engines that don't fold to upper-case.

isthmus hard-codes Casing.TO_UPPER. That's the ANSI fold direction and Calcite's default, but a large class of real engines fold unquoted identifiers to lower-case or preserve them — Presto/Trino, Spark, DuckDB, Postgres. So a plan generated from SELECT … FROM orders comes out with a NamedScan named ORDERS and columns like O_ORDERKEY. When that plan is handed to a lower-casing engine, none of the table/column names match what the engine actually has.

This isn't hypothetical — I ran into it head-on while prototyping a native Substrait consumer for Presto (Substrait → Presto PlanNode IR directly, no SQL bridge; still WIP on a branch in my fork). Presto normalizes unquoted identifiers to lower-case, so isthmus-generated plans came in upper-cased and didn't resolve. To make it work I had to:

• add case-insensitive name resolution + a lower-casing bridge in the plan converter (the NamedScan handling ended up with a comment: "field names may differ in case from Presto's (lower-cased) column names"), and
• deliberately write the TPC-H/TPC-DS test queries with unquoted names so isthmus's and Presto's identifier casing would line up at all.

The integration itself isn't upstream and isn't a dependency here — I'm raising it only as evidence that the mismatch is real: any consumer targeting a lower-casing engine has to reinvent that bridge, purely because the plan-generation side has no way to say "don't upper-case."

The examples already demonstrate the same inconsistency. FromSql generates a plan for DuckDB and needs the identifiers left as written — it was leaning on the deprecated convert(sql, catalog, SqlDialect) to get there. With casing configurable it just asks for Casing.UNCHANGED and the lower-case names survive end to end.

On "push users to parse in Calcite themselves": for most of SqlParser.Config, I agree completely. But these are precisely the consumers who do want isthmus's SQL front-end rather than hand-rolling Calcite — the isthmus CLI, the examples, and plan-generation/test harnesses. In fact the CLI already ships a first-class --unquotedcasing flag, so casing control is an established, supported isthmus feature. The only way IsthmusEntryPoint can implement it today is the anonymous ConverterProvider subclass overriding getSqlParserConfig() — which is exactly the workaround this PR removes. So the inconsistency is internal too: the CLI exposes casing, but the library API forces everyone else to duplicate that subclass boilerplate for the one setting we hit constantly.

On the API shape (your inline comment): your flag-creep worry is legitimate, and I don't want to relitigate the history of bolting individual parser toggles onto the API — getSqlParserConfig() returning a full config should stay the extension point for arbitrary Calcite parser settings. My argument is that casing is not just a parse-time setting the way most of SqlParser.Config is: it leaks into the output. It determines the identifier names written into the emitted Substrait plan, and therefore whether the plan is portable to the target engine at all. That's why it seems worth a first-class knob while everything else stays behind the override.

Happy to reshape to whatever you're comfortable with. A couple of options:

1. Keep it minimal / non-breaking — drop the constructor param and the removal of the SqlParser.Config overloads, and just document + use the getSqlParserConfig() override pattern in the CLI/examples. (Keeps the boilerplate, but no API surface change.)
2. First-class unquotedCasing as in this PR — because it's the one parser option that changes plan semantics, not just parsing — but I can keep the SqlParser.Config overloads so it isn't a breaking change.

I'd lean toward (2)-without-the-breaking-removal, but I'm glad to go with (1) if you'd rather not grow ConverterProvider's surface. What's your preference?

[vbarua]

TL;DR: isthmus hard-codes Casing.TO_UPPER, so plans it generates carry upper-cased identifiers (ORDERS, O_ORDERKEY) that don't match engines which fold to lower-case (Presto/Trino, Spark, DuckDB, Postgres).

So what you're saying is that if we generate plans with upper-cased identifiers, that only works on some systems, and the same thing with lower-cased identifiers. This sounds like a deeper problem with the spec, because now we have plans that are basically identical except for the casing of names that don't work across engines. Making it easier to control the casing in the output papers over the issue here.

[nielspardon]

This sounds like a deeper problem with the spec, because now we have plans that are basically identical except for the casing of names that don't work across engines. Making it easier to control the casing in the output papers over the issue here.

From my perspective names in the spec are case-sensitive aka quoted identifiers in SQL-speak. We only have the problem when we parse SQL statements into Substrait plans. Currently, the SqlParser.Config is not aligned across the create statement parsing which creates the Calcite catalog and the other SQL to Substrait parsing. Further we are testing round-tripping by parsing SQL statements using unquoted identifiers in SQL and not quoted identifiers. Our users might have the same problem that they want to parse SQL with unquoted identifiers into Substrait plans so they very frequently want to configure the behavior for how to handle unquoted identifiers when parsing SQL statements.

[vbarua]

From my perspective names in the spec are case-sensitive aka quoted identifiers in SQL-speak.

That does feel reasonable. Anything else is a can-of-worms, and we might want to open it one day but it doesn't need to be today.

We only have the problem when we parse SQL statements into Substrait plans. Currently, the SqlParser.Config is not aligned across the create statement parsing which creates the Calcite catalog and the other SQL to Substrait parsing.

Gotcha, from this I think it's perfectly reasonable to inject the ConvertProvider into the statement parsers.

Our users might have the same problem that they want to parse SQL with unquoted identifiers into Substrait plans so they very frequently want to configure the behavior for how to handle unquoted identifiers when parsing SQL statements.

How are we expecting users to parse SQL? Are they doing so through the isthmus CLI, or are they integrating with the full Java library?

[nielspardon]

How are we expecting users to parse SQL? Are they doing so through the isthmus CLI, or are they integrating with the full Java library?

I would say both currently. For the Presto integration I used isthmus (the lib) to take their existing SQL test suite and create Substrait plans on the fly which are in Presto SQL dialect and may or may not use unquoted identifiers. We are also doing that for some of our internal work with a number of pre-existing SQL based benchmarks which may have been created with various SQL dialects.

The CLI is then just a thin wrapper around the lib which allows one to use a subset of the isthmus features without requiring Java.