rad-pat opened a new pull request, #43964: URL: https://github.com/apache/superset/pull/43964
### SUMMARY `SQLGLOT_DIALECTS` carried `# "databend": ???`, so Databend queries fell back to the generic dialect. Superset regenerates every adhoc column and metric through that dialect (`sanitize_clause`, called from `_process_sql_expression` in `superset/models/helpers.py`), so whatever the fallback renders reaches the server on every compile. This adds a dialect and fills in that entry. **Why Postgres and not ClickHouse.** ClickHouse looks like the natural base — Databend borrows much of its surface syntax, including `SETTINGS` and the `to_start_of_*` date helpers — but its *generator* renames a great many functions to spellings Databend does not have. Round-tripping every function name ClickHouse's parser knows produced 156 renames, of which **44 emit a name absent from Databend's catalogue**: `argMax`, `countIf`, `stddevSamp`, `varSamp`, `splitByString`, `JSONExtractString`, `toTypeName`, `editDistance`, `lagInFrame`, `arrayJoin`, the whole `array*` camelCase family, and `POSITION(x, y)` where Databend accepts only `POSITION(x IN y)`. On a 70-expression battery: | candidate base | expressions broken | | --- | --- | | ClickHouse | 33 | | **Postgres (this PR)** | **17** | | MySQL | 17 | | DuckDB | 23 | Postgres is also what `databend-sqlalchemy` already assumes: its `DatabendCompiler` and `DatabendIdentifierPreparer` derive from `PGCompiler` and `PGIdentifierPreparer`, so the SQL Superset compiles and the SQL this dialect regenerates come from the same family rather than disagreeing. **What the dialect adds on top of Postgres:** - Databend's *leading* `SETTINGS (...)` clause (e.g. `SETTINGS (max_execute_time_in_seconds=300) SELECT ...`), which no built-in dialect parses — absorbed and re-emitted verbatim. - ClickHouse's *trailing* `SETTINGS k = v`, which Databend also accepts. This needs `SETTINGS` kept out of `TABLE_ALIAS_TOKENS`, or `FROM t SETTINGS ...` binds it as the table alias. - Backtick identifiers alongside Postgres's double-quoted form. - 16 `TRANSFORMS` overrides for names the Postgres base itself gets wrong. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable — no UI change. ### TESTING INSTRUCTIONS ``` pytest tests/unit_tests/sql/dialects/databend_tests.py ``` 57 tests, covering the `SETTINGS` forms (leading, trailing, and the retreat path for a bare `SETTINGS` keyword), one case per `TRANSFORMS` override with the exact spelling asserted, cast rendering, and the enumerating sweep described below. Every override was checked against a **live Databend (Query v1.2.790)** rather than inferred. That caught three things static analysis missed: - Postgres's `~` regex operator is rejected — hence the `RegexpLike` override. - `= ANY(array)` is rejected — hence the `ArrayContains` override. - `CURRENT_DATE` is rejected **both** with and without parentheses; Databend wants `TODAY()`. Also confirmed live: Postgres-style casts (`TEXT`, `BIGINT`, `DOUBLE PRECISION`) are accepted, and the existing `superset/db_engine_specs/databend.py` time-grain expressions (`to_start_of_*`, `to_monday`) survive the round-trip unchanged. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [x] Introduces new feature or API - [ ] Removes existing feature or API **A note on the test fixture, which I am happy to drop.** `tests/unit_tests/sql/dialects/databend_function_catalogue.txt` is Databend's function catalogue captured from `SHOW FUNCTIONS`, and `test_no_transform_emits_a_function_databend_lacks` uses it to round-trip every parser-known function and assert that any rewrite lands on a name Databend actually has. It sweeps 582 names and reports 82 failures against a ClickHouse base and none against this one, so a future sqlglot bump cannot introduce a divergence unnoticed — a hand-written battery is precisely what let 44 of them hide. I recognise this is unlike the other dialect tests here, which are all explicit input/expected pairs, and that a checked-in vendor catalogue will drift as Databend adds functions (the file carries a header saying how to regenerate it). If you would rather not carry it, deleting that test and the fixture leaves `test_transform_overrides_emit_databend_spellings` — 16 parametrized cases in the same idiom as `db2_tests.py` and `firebolt_tests.py` — and the PR stands on its own. Happy to cut it on request. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
