feat(sql): wire an actual SQL dialect selector, enable ClickHouse parsing - #147
Merged
Merged
Conversation
Unrelated formatting drift caught by the workspace-wide cargo-fmt pre-commit hook while preparing an unrelated change. No behavior change.
…sing `SqlDialect::ClickhouseSQL` existed on `QueryLanguage::SQL` but `lower_sql_batch` unconditionally rejected it with `UnsupportedDialect`, and `lower_sql` never consulted the dialect at all -- DataFusion's default GenericDialect was always used. sqlparser (already a transitive dep via DataFusion) vendors a `ClickHouseDialect` that parses ClickHouse's array-lambda syntax (`arrayFilter(x -> ..., ...)`) and `arr[-1]` indexing, neither of which GenericDialect accepts. Adds `SqlLowerer::with_dialect` / `lower_sql_dialect`, threads the dialect into DataFusion's `SessionConfig` (`datafusion.sql_parser.dialect`), and auto-registers a schema for dotted catalog table names (e.g. `bgp.bgp_updates`) since DataFusion requires the parent schema to exist before a qualified table can be registered into it. `lower_sql_batch` now only rejects `ElasticSQL`, which has no vendored parser. This is parsing-only: ClickHouse-specific builtin functions (`uniqExact`, `countIf`, `toStartOfInterval`, ...) are still unknown to DataFusion's planner and still fail to plan -- registering UDF equivalents is separate follow-up work, not attempted here.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR turns on ClickHouse SQL parsing in the SQL front end, which today silently ignores the requested dialect and always parses with DataFusion's generic SQL grammar. It's prep for testing real ClickHouse-flavored query corpora (like the BGP analytics one in #148) against the lowering pipeline.
Summary
SqlDialect::ClickhouseSQLexisted onQueryLanguage::SQL(_)butlower_sql_batchunconditionally rejected it withUnsupportedDialect, andlower_sqlnever consulted the dialect at all -- DataFusion's defaultGenericDialectwas always used regardless.ClickHouseDialectthat parses ClickHouse's array-lambda syntax (arrayFilter(x -> ..., ...)) andarr[-1]indexing, neither of whichGenericDialectaccepts. No new dependency.SqlLowerer::with_dialect/lower_sql_dialect, threads the dialect into DataFusion'sSessionConfig(datafusion.sql_parser.dialect), and auto-registers a schema for dotted catalog table names (e.g.bgp.bgp_updates) since DataFusion requires the parent schema to exist before a qualified table can be registered into it.lower_sql_batchnow only rejectsElasticSQL, which has no vendored parser at all.lower_sqlkeeps its existing signature/behavior (delegates tolower_sql_dialectwithSqlDialect::DataFusionSQL) -- no breaking change for existing callers/tests.cargo fmtdrift fix (cost_model.rs), split into its own commit, needed to get the workspace-wide fmt pre-commit hook green.Scope note
This is parsing-only. ClickHouse-specific builtin functions (
uniqExact,countIf,toStartOfInterval,lagInFrame,isIPAddressInRange,multiIf,arrayJoin,groupUniqArray, ...) are still unknown to DataFusion's planner and still fail to plan. Registering UDF equivalents for those is separate follow-up work, not attempted here.Stack
Base for #148, which exercises this dialect switch against a real-world ClickHouse query corpus.
Test plan
cargo build --workspacecargo test -p asap-frontend-sql(existing suite green, 65 tests)cargo clippy -p asap-frontend-sql --all-targets-- clean🤖 Generated with Claude Code