You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rad-pat (via GitHub)" <gi...@apache.org> on 2026/09/07 12:55:31 UTC
[PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rad-pat (via GitHub)" <gi...@apache.org>.
rad-pat commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5635955533
Correcting myself: I said above that a re-run should be enough. It isn't — `unit-tests (current)` hit the cap a second time, so this is not a flake and I should not have called it one.
Second run, all three pytest invocations green, then killed 5 seconds after the last one finished:
```
=========== 15307 passed, 5 skipped, 2 xfailed in 1568.97s (0:26:08) ===========
============================ 1898 passed in 35.93s =============================
============================= 437 passed in 32.48s =============================
##[error]The operation was canceled.
```
30m51s against `timeout-minutes: 30` in `superset-python-unittest.yml`. Test time alone is 27m17s across the three runs, so setup plus install eats the rest of the budget.
I pulled the JUnit artifact to check this branch isn't the cause. It isn't — the new file is **4.1s of 1470.5s of test time, 0.28%**:
```
4.1s 181 tests 0.28% tests/unit_tests/sql/dialects/databend_tests
1.1s 54 tests 0.08% tests/unit_tests/db_engine_specs/test_databend
1.1s 17 tests 0.08% tests/unit_tests/migrations/test_databend_secure_to_sslmode
```
Deleting the whole thing would buy 4 seconds of the 51 it is over by. For context, the ten most expensive files are 200s between them and most are `mcp_service`; no single test reaches the 0.5s `--durations-min` threshold, so there is no hot spot to fix — the suite has simply grown into the cap.
Happy to trim anything here if it helps, but I don't think it would. This looks like it wants `timeout-minutes` raised or the run split, which is a maintainer call rather than mine. Flagging it because any Python PR opened this week will be landing on the same wall.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5594143981
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5628620861
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5580211062
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5571035640
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43964?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `51.21951%` with `20 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.54%. Comparing base ([`53070fa`](https://app.codecov.io/gh/apache/superset/commit/53070fa4c04c192118cb393c3e5bbe68eb1abbcb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`01ba1f3`](https://app.codecov.io/gh/apache/superset/commit/01ba1f301d24f0481c2547eeca6d5b20dcda0e4b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/43964?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/sql/dialects/databend.py](https://app.codecov.io/gh/apache/superset/pull/43964?src=pr&el=tree&filepath=superset%2Fsql%2Fdialects%2Fdatabend.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvc3FsL2RpYWxlY3RzL2RhdGFiZW5kLnB5) | 50.00% | [20 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43964?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (53070fa) and HEAD (01ba1f3). Click for more details.
>
> <details><summary>HEAD has 17 uploads less than BASE</summary>
>
>| Flag | BASE (53070fa) | HEAD (01ba1f3) |
>|------|------|------|
>|python|15|2|
>|presto|3|1|
>|hive|3|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43964 +/- ##
===========================================
- Coverage 79.81% 57.54% -22.27%
===========================================
Files 2914 2915 +1
Lines 170467 170508 +41
Branches 39518 39521 +3
===========================================
- Hits 136060 98126 -37934
- Misses 31885 71460 +39575
+ Partials 2522 922 -1600
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.64% <51.21%> (+<0.01%)` | :arrow_up: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.52% <51.21%> (+<0.01%)` | :arrow_up: |
| [python](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.57% <51.21%> (-44.63%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/43964/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43964?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43964:
URL: https://github.com/apache/superset/pull/43964#discussion_r3949958632
##########
superset/sql/dialects/databend.py:
##########
@@ -0,0 +1,164 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Databend dialect.
+
+Databend has no built-in sqlglot dialect, so ``SQLGLOT_DIALECTS`` had no entry for
+it and every Databend query fell back to the generic dialect. Superset regenerates
+each adhoc column and metric through that dialect (``sanitize_clause``, called from
+``_process_sql_expression`` in ``superset/models/helpers.py``), so the fallback's
+rendering reached the server on every compile.
+
+The base is Postgres, which matches ``databend-sqlalchemy``: 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.
+
+ClickHouse looks like the closer fit -- Databend borrows much of its surface
+syntax, including ``SETTINGS`` and the ``to_start_of_*`` date helpers -- but it is
+not. ClickHouse's generator renames a large number of functions to spellings
+Databend does not have. Round-tripping every function name its parser knows
+produced 156 renames, 44 of which emitted a name absent from Databend's catalogue:
+``argMax``, ``countIf``, ``stddevSamp``, ``splitByString``, ``JSONExtractString``,
+``toTypeName``, ``lagInFrame``, ``arrayJoin``, the ``array*`` camelCase family, and
+``POSITION(x, y)`` where Databend accepts only ``POSITION(x IN y)``. On a
+70-expression battery a ClickHouse base broke 33 where Postgres breaks 17.
+
+What Postgres still gets wrong is overridden below, and a regression test
+enumerates the transforms so the list cannot grow unnoticed on a sqlglot bump.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Callable
+
+from sqlglot import exp, generator
+from sqlglot.dialects.dialect import rename_func
+from sqlglot.dialects.postgres import Postgres
+from sqlglot.tokens import TokenType
+
+
+def _date_format(args: list[Any]) -> exp.Expression:
+ """
+ Parse ``formatDateTime(...)`` as Databend's ``DATE_FORMAT(...)``.
+
+ Databend accepts ClickHouse's ``formatDateTime`` spelling nowhere -- "no
+ function matches the given name: 'formatdatetime', do you mean 'date_format'?"
+ -- but expressions carrying it turn up in datasets migrated from ClickHouse.
+ Both take the same ``%``-style format string, so it is passed along untouched.
+ """
+ return exp.Anonymous(this="DATE_FORMAT", expressions=args)
+
+
+def _sha2(self: generator.Generator, expression: exp.SHA2) -> str:
+ """Databend spells this ``SHA2(x, 256)``; Postgres emits ``SHA256(x)``."""
+ return self.func("SHA2", expression.this, expression.args.get("length"))
+
+
+class Databend(Postgres):
+ class Tokenizer(Postgres.Tokenizer):
+ # Databend accepts backtick-quoted identifiers as well as the
+ # double-quoted form Postgres uses.
+ IDENTIFIERS = ['"', "`"]
+
+ class Parser(Postgres.Parser):
+ FUNCTIONS: dict[str, Callable[..., exp.Expression]] = {
+ **Postgres.Parser.FUNCTIONS,
+ "FORMATDATETIME": _date_format,
+ }
+
+ # Without this, ``FROM t SETTINGS max_threads = 1`` binds SETTINGS as the
+ # table's alias and the assignment that follows fails to parse.
+ TABLE_ALIAS_TOKENS = Postgres.Parser.TABLE_ALIAS_TOKENS - {TokenType.SETTINGS}
+
+ # Databend accepts ClickHouse's trailing ``SETTINGS k = v``.
+ QUERY_MODIFIER_PARSERS = {
+ **Postgres.Parser.QUERY_MODIFIER_PARSERS,
+ TokenType.SETTINGS: lambda self: (
+ "settings",
+ self._advance() or self._parse_csv(self._parse_assignment),
+ ),
+ }
+
+ def _parse_statement(self) -> exp.Expression | None:
+ # Databend also accepts a *leading* ``SETTINGS (...)`` clause before
+ # the statement -- e.g.
+ # ``SETTINGS (max_execute_time_in_seconds=300) SELECT ...`` -- which no
+ # built-in dialect parses. It is absorbed here and re-emitted verbatim
+ # in ``Generator.generate``.
+ settings = None
+ if self._curr and self._curr.token_type == TokenType.SETTINGS:
+ index = self._index
+ start = self._curr
+ self._advance()
+ if self._curr and self._curr.token_type == TokenType.L_PAREN:
+ self._parse_wrapped_csv(self._parse_assignment)
+ settings = self._find_sql(start, self._prev)
Review Comment:
**Suggestion:** The parsed leading assignments are discarded, so `SQLStatement.get_settings()` and `SQLScript.get_settings()` cannot report values from leading `SETTINGS (...)` clauses. [api mismatch]
**Assessment:** 🟠 `Major` · 🔁 `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=22bb85ab7ed44c3a9ddec28017de84bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=22bb85ab7ed44c3a9ddec28017de84bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/sql/dialects/databend.py
**Line:** 111:111
**Comment:**
*Api Mismatch: The parsed leading assignments are discarded, so `SQLStatement.get_settings()` and `SQLScript.get_settings()` cannot report values from leading `SETTINGS (...)` clauses.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43964&comment_hash=a3c6f303b912710f08814056bfb717fc2f31b979d7d75bc14107db28a9137c65&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43964&comment_hash=a3c6f303b912710f08814056bfb717fc2f31b979d7d75bc14107db28a9137c65&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5571015639
<!-- Bito Reply -->
The issue is correct. The current implementation of `_parse_statement` in `superset/sql/dialects/databend.py` absorbs the leading `SETTINGS` clause and attaches it to the statement, but it does not expose this information to the `get_settings()` methods of `SQLStatement` or `SQLScript`.
To resolve this, you should update `superset/sql/parse.py` to extract the `leading_settings` argument from the parsed expression and include it in the settings returned by `get_settings()`. Since the `leading_settings` are stored as an `exp.Expression` (specifically the result of `_find_sql`), you will need to parse or extract the key-value pairs from that expression to match the format expected by `get_settings()`.
I have validated the issue and the proposed approach. Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?
**superset/sql/dialects/databend.py**
```
def _parse_statement(self) -> exp.Expression | None:
# ... (existing logic)
statement = super()._parse_statement()
if settings and statement:
statement.set("leading_settings", settings)
return statement
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rad-pat (via GitHub)" <gi...@apache.org>.
rad-pat commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5571367289
Thanks — the `get_settings()` observation pointed at something real, though the fix went a different way than suggested. I ran an adversarial review over the branch as well, and it found two further defects. All three are fixed and the branch is force-pushed (still one commit).
**1. JSON paths were silently corrupted.** `Postgres.Generator.TRANSFORMS[exp.JSONExtract]` is not a plain renamer — it explodes a path into one argument per segment, `JSON_EXTRACT_PATH(j, 'a', 'b')`. Databend instead takes exactly two arguments with the path as a single dotted string. Replacing the transform with a bare `rename_func` concatenated the segments:
```sql
-- before
SELECT JSON_EXTRACT(j, '$.a.b') -> SELECT JSON_EXTRACT_PATH_TEXT(j, 'ab')
-- after
SELECT JSON_EXTRACT(j, '$.a.b') -> SELECT JSON_EXTRACT_PATH_TEXT(j, 'a.b')
```
`'ab'` is a key that does not exist, so the server returned NULL rather than raising — silent wrong results on any nested JSON access. Now rebuilt properly, including subscripts (`'a[0].b'`), with a fallback to `JSON_PATH_QUERY_FIRST` when the path is an expression rather than a literal. Postgres's `->` and `->>` operators are covered by the same path.
**2. `settings` was silently dropped as a table alias.** Removing `SETTINGS` from `TABLE_ALIAS_TOKENS` (needed so `FROM t SETTINGS max_threads = 1` parses) meant a table aliased `settings` lost its alias with no error, leaving qualified references dangling:
```sql
-- before
SELECT a.x FROM t settings JOIN u ON settings.x = u.x
-> SELECT a.x FROM t JOIN u ON settings.x = u.x -- alias gone, reference dangling
```
The clause and the alias are now told apart by looking ahead for the `=` that only the clause has — the same way the base parser disambiguates `LIMIT` and `OFFSET` in `_parse_table_alias`.
**3. The leading `SETTINGS (...)` clause now accepts only constant values.** This is the one your comment was circling. Rather than surfacing the assignments through `get_settings()`, I tightened what may be absorbed at all, because the underlying risk was larger than an API gap: the clause is re-emitted from source text rather than regenerated from the tree, so anything the parser swallowed without exposing as tree nodes was invisible to the table extraction behind RLS and the denylist, yet still forwarded to the server verbatim. `_parse_assignment` accepted a subquery, so `SETTINGS (a = (SELECT x FROM secret)) SELECT 1 FROM public` parsed with only `public` visible to `find_all(exp.Table)`.
Databend rejects a non-constant there anyway ("value must be constant value"), so refusing to absorb it costs nothing and keeps the tree an honest account of the statement. Note the *trailing* `SETTINGS k = v` form never had this problem — it keeps real parsed nodes — so the two are now consistent.
Each replacement and each of the three behaviours above was checked against a live Databend (Query v1.2.790) rather than inferred. Tests went from 57 to 77; the new ones assert per path shape and per alias form, since the enumerating sweep compares function-name spellings and would not have caught any of this.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43964:
URL: https://github.com/apache/superset/pull/43964#discussion_r4060577650
##########
superset/sql/dialects/databend.py:
##########
@@ -0,0 +1,164 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Databend dialect.
+
+Databend has no built-in sqlglot dialect, so ``SQLGLOT_DIALECTS`` had no entry for
+it and every Databend query fell back to the generic dialect. Superset regenerates
+each adhoc column and metric through that dialect (``sanitize_clause``, called from
+``_process_sql_expression`` in ``superset/models/helpers.py``), so the fallback's
+rendering reached the server on every compile.
+
+The base is Postgres, which matches ``databend-sqlalchemy``: 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.
+
+ClickHouse looks like the closer fit -- Databend borrows much of its surface
+syntax, including ``SETTINGS`` and the ``to_start_of_*`` date helpers -- but it is
+not. ClickHouse's generator renames a large number of functions to spellings
+Databend does not have. Round-tripping every function name its parser knows
+produced 156 renames, 44 of which emitted a name absent from Databend's catalogue:
+``argMax``, ``countIf``, ``stddevSamp``, ``splitByString``, ``JSONExtractString``,
+``toTypeName``, ``lagInFrame``, ``arrayJoin``, the ``array*`` camelCase family, and
+``POSITION(x, y)`` where Databend accepts only ``POSITION(x IN y)``. On a
+70-expression battery a ClickHouse base broke 33 where Postgres breaks 17.
+
+What Postgres still gets wrong is overridden below, and a regression test
+enumerates the transforms so the list cannot grow unnoticed on a sqlglot bump.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Callable
+
+from sqlglot import exp, generator
+from sqlglot.dialects.dialect import rename_func
+from sqlglot.dialects.postgres import Postgres
+from sqlglot.tokens import TokenType
+
+
+def _date_format(args: list[Any]) -> exp.Expression:
+ """
+ Parse ``formatDateTime(...)`` as Databend's ``DATE_FORMAT(...)``.
+
+ Databend accepts ClickHouse's ``formatDateTime`` spelling nowhere -- "no
+ function matches the given name: 'formatdatetime', do you mean 'date_format'?"
+ -- but expressions carrying it turn up in datasets migrated from ClickHouse.
+ Both take the same ``%``-style format string, so it is passed along untouched.
+ """
+ return exp.Anonymous(this="DATE_FORMAT", expressions=args)
+
+
+def _sha2(self: generator.Generator, expression: exp.SHA2) -> str:
+ """Databend spells this ``SHA2(x, 256)``; Postgres emits ``SHA256(x)``."""
+ return self.func("SHA2", expression.this, expression.args.get("length"))
+
+
+class Databend(Postgres):
+ class Tokenizer(Postgres.Tokenizer):
+ # Databend accepts backtick-quoted identifiers as well as the
+ # double-quoted form Postgres uses.
+ IDENTIFIERS = ['"', "`"]
+
+ class Parser(Postgres.Parser):
+ FUNCTIONS: dict[str, Callable[..., exp.Expression]] = {
+ **Postgres.Parser.FUNCTIONS,
+ "FORMATDATETIME": _date_format,
+ }
+
+ # Without this, ``FROM t SETTINGS max_threads = 1`` binds SETTINGS as the
+ # table's alias and the assignment that follows fails to parse.
+ TABLE_ALIAS_TOKENS = Postgres.Parser.TABLE_ALIAS_TOKENS - {TokenType.SETTINGS}
+
+ # Databend accepts ClickHouse's trailing ``SETTINGS k = v``.
+ QUERY_MODIFIER_PARSERS = {
+ **Postgres.Parser.QUERY_MODIFIER_PARSERS,
+ TokenType.SETTINGS: lambda self: (
+ "settings",
+ self._advance() or self._parse_csv(self._parse_assignment),
+ ),
+ }
+
+ def _parse_statement(self) -> exp.Expression | None:
+ # Databend also accepts a *leading* ``SETTINGS (...)`` clause before
+ # the statement -- e.g.
+ # ``SETTINGS (max_execute_time_in_seconds=300) SELECT ...`` -- which no
+ # built-in dialect parses. It is absorbed here and re-emitted verbatim
+ # in ``Generator.generate``.
+ settings = None
+ if self._curr and self._curr.token_type == TokenType.SETTINGS:
+ index = self._index
+ start = self._curr
+ self._advance()
+ if self._curr and self._curr.token_type == TokenType.L_PAREN:
+ self._parse_wrapped_csv(self._parse_assignment)
+ settings = self._find_sql(start, self._prev)
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not require get_settings() to include leading query-scoped SETTINGS clauses in the Databend dialect; reserve it for statement-form SET assignments that can rebind session state across statements.
**Applied to:**
- `superset/sql/dialects/databend.py`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5760496236
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #43964:
URL: https://github.com/apache/superset/pull/43964
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rad-pat (via GitHub)" <gi...@apache.org>.
rad-pat commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5632753644
`unit-tests (current)` shows red but nothing failed — the job hit the 30-minute cap during teardown, after both pytest runs had finished green:
```
=========== 15301 passed, 5 skipped, 2 xfailed in 1567.45s (0:26:07) ===========
...
superset/sql/dialects/databend.py 114 0 36 0 100%
Required test coverage of 100% reached. Total coverage: 100.00%
============================ 1892 passed in 34.84s =============================
##[error]The operation was canceled.
```
The job ran 08:54:42 → 09:25:00, exactly 30m18s ([run](https://github.com/apache/superset/actions/runs/34581428679)). A successful run of the same job on another PR this morning took 29m53s, so the step is sitting right on the limit for everyone at the moment rather than because of anything here — `--durations-min=0.5` lists nothing from the new test file, and the whole `tests/unit_tests/sql/` suite including it runs in 34.84s.
A re-run should be enough; I have not force-pushed again just to re-roll it. Everything else on this head is green apart from the `superset-docs-preview` Netlify checks, which were already failing before this branch.
The PR description has also been rewritten — it had gone stale against the commit.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5587240825
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rad-pat (via GitHub)" <gi...@apache.org>.
rad-pat commented on code in PR #43964:
URL: https://github.com/apache/superset/pull/43964#discussion_r4060574866
##########
superset/sql/dialects/databend.py:
##########
@@ -0,0 +1,164 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Databend dialect.
+
+Databend has no built-in sqlglot dialect, so ``SQLGLOT_DIALECTS`` had no entry for
+it and every Databend query fell back to the generic dialect. Superset regenerates
+each adhoc column and metric through that dialect (``sanitize_clause``, called from
+``_process_sql_expression`` in ``superset/models/helpers.py``), so the fallback's
+rendering reached the server on every compile.
+
+The base is Postgres, which matches ``databend-sqlalchemy``: 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.
+
+ClickHouse looks like the closer fit -- Databend borrows much of its surface
+syntax, including ``SETTINGS`` and the ``to_start_of_*`` date helpers -- but it is
+not. ClickHouse's generator renames a large number of functions to spellings
+Databend does not have. Round-tripping every function name its parser knows
+produced 156 renames, 44 of which emitted a name absent from Databend's catalogue:
+``argMax``, ``countIf``, ``stddevSamp``, ``splitByString``, ``JSONExtractString``,
+``toTypeName``, ``lagInFrame``, ``arrayJoin``, the ``array*`` camelCase family, and
+``POSITION(x, y)`` where Databend accepts only ``POSITION(x IN y)``. On a
+70-expression battery a ClickHouse base broke 33 where Postgres breaks 17.
+
+What Postgres still gets wrong is overridden below, and a regression test
+enumerates the transforms so the list cannot grow unnoticed on a sqlglot bump.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Callable
+
+from sqlglot import exp, generator
+from sqlglot.dialects.dialect import rename_func
+from sqlglot.dialects.postgres import Postgres
+from sqlglot.tokens import TokenType
+
+
+def _date_format(args: list[Any]) -> exp.Expression:
+ """
+ Parse ``formatDateTime(...)`` as Databend's ``DATE_FORMAT(...)``.
+
+ Databend accepts ClickHouse's ``formatDateTime`` spelling nowhere -- "no
+ function matches the given name: 'formatdatetime', do you mean 'date_format'?"
+ -- but expressions carrying it turn up in datasets migrated from ClickHouse.
+ Both take the same ``%``-style format string, so it is passed along untouched.
+ """
+ return exp.Anonymous(this="DATE_FORMAT", expressions=args)
+
+
+def _sha2(self: generator.Generator, expression: exp.SHA2) -> str:
+ """Databend spells this ``SHA2(x, 256)``; Postgres emits ``SHA256(x)``."""
+ return self.func("SHA2", expression.this, expression.args.get("length"))
+
+
+class Databend(Postgres):
+ class Tokenizer(Postgres.Tokenizer):
+ # Databend accepts backtick-quoted identifiers as well as the
+ # double-quoted form Postgres uses.
+ IDENTIFIERS = ['"', "`"]
+
+ class Parser(Postgres.Parser):
+ FUNCTIONS: dict[str, Callable[..., exp.Expression]] = {
+ **Postgres.Parser.FUNCTIONS,
+ "FORMATDATETIME": _date_format,
+ }
+
+ # Without this, ``FROM t SETTINGS max_threads = 1`` binds SETTINGS as the
+ # table's alias and the assignment that follows fails to parse.
+ TABLE_ALIAS_TOKENS = Postgres.Parser.TABLE_ALIAS_TOKENS - {TokenType.SETTINGS}
+
+ # Databend accepts ClickHouse's trailing ``SETTINGS k = v``.
+ QUERY_MODIFIER_PARSERS = {
+ **Postgres.Parser.QUERY_MODIFIER_PARSERS,
+ TokenType.SETTINGS: lambda self: (
+ "settings",
+ self._advance() or self._parse_csv(self._parse_assignment),
+ ),
+ }
+
+ def _parse_statement(self) -> exp.Expression | None:
+ # Databend also accepts a *leading* ``SETTINGS (...)`` clause before
+ # the statement -- e.g.
+ # ``SETTINGS (max_execute_time_in_seconds=300) SELECT ...`` -- which no
+ # built-in dialect parses. It is absorbed here and re-emitted verbatim
+ # in ``Generator.generate``.
+ settings = None
+ if self._curr and self._curr.token_type == TokenType.SETTINGS:
+ index = self._index
+ start = self._curr
+ self._advance()
+ if self._curr and self._curr.token_type == TokenType.L_PAREN:
+ self._parse_wrapped_csv(self._parse_assignment)
+ settings = self._find_sql(start, self._prev)
Review Comment:
The observation is factually right and the severity isn't: leading `SETTINGS (...)` values genuinely don't appear in `get_settings()`, but neither consumer of that API can be affected by it.
`get_settings()` collects `exp.SetItem` nodes — the `SET foo = 'bar'` **statement** form. It has two callers in `superset/sql/parse.py`, both about session rebinding:
- `changes_search_path()` (:1308) looks for a `search_path` key, so that denylist matching against `default_schema` can be treated as unreliable afterwards.
- the rebinding check (:1359) looks for `schema` / `current_schema` / `current schema` / `catalog`.
Databend's leading `SETTINGS (...)` is a query-scoped wrapper on a single statement — `SETTINGS (max_execute_time_in_seconds=300) SELECT ...`. It cannot rebind resolution for *later* statements, which is the premise of both checks, and the clause is restricted here to constant `key = literal` pairs, so nothing else can ride in on it.
The risk that restriction exists for is the one worth naming, because it is adjacent to yours: the clause is re-emitted from source text rather than regenerated from the tree, so anything the parser swallowed without exposing as tree nodes would be invisible to table extraction while still reaching the server. That is closed by `_is_constant_assignment` checking **both** sides of each pair — an earlier revision only checked the value, and `SETTINGS ((SELECT x FROM secret) = 1) SELECT a FROM t` was absorbed with the subquery intact while the tree reported one table. It is now rejected, with `test_leading_settings_rejects_non_constant_values` covering both sides and `test_nothing_absorbed_is_missing_from_the_tree` asserting the property directly: every table named in the re-emitted statement is reachable through the tree.
Same conclusion @rusackas reached in the approving review; I have re-derived it from the call sites rather than restating it. Resolving on that basis — happy to reopen if anyone reads those two call sites differently.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5570954617
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "rad-pat (via GitHub)" <gi...@apache.org>.
rad-pat commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5582102794
Suggestion 1 was a real bug and is fixed. The other two I've looked into and am leaving as they are, with reasoning below.
**1. JSON path segments dropped — correct, and my fault.** I had a fallback for exactly this and removed it in the previous push, on the mistaken basis that sqlglot rejects such paths before generation. That is only true of the *rooted* spelling, which is the single case I tested:
```
$..b ValueError from sqlglot <- what I checked
a..b JSON_EXTRACT_PATH_TEXT(j, 'a') <- reaches the dialect, segment dropped
a[?(@.x)] JSON_EXTRACT_PATH_TEXT(j, 'a')
a[1,2] JSON_EXTRACT_PATH_TEXT(j, 'a')
```
So the unrooted forms silently read the wrong field and return NULL — the same failure as the segment concatenation this PR set out to fix, one level down. The fallback is restored, and now that these inputs are known to reach it, they are covered by a test rather than being an unreachable branch. Paths that cannot be expressed as a Databend dotted path now go to `JSON_PATH_QUERY_FIRST`, and where sqlglot cannot render the path either, it raises — the right outcome for a path this dialect cannot faithfully express, and much better than quietly returning a different field.
**2. Pass-throughs not flagged.** Deliberate, though worth stating explicitly. The guard exists to catch a *rewrite* landing on a name Databend lacks — a defect in this dialect. A name that passes through untouched and that Databend does not have is a user writing a function their database does not provide, which is not something a dialect should be asserting about; every dialect here would fail such a check. The docstring claims only the narrower thing. Flagging pass-throughs would fail on the ~436 names that round-trip unchanged, most of which are other engines' spellings that Databend legitimately lacks.
**3. `rlike` in the catalogue.** Confirmed on a live server — it is in `SHOW FUNCTIONS` but is not callable, and Databend answers `rlike('a', 'b')` with "missing lhs or rhs for the binary operator". I have left it in, because the file is that query's output verbatim and hand-removing entries would make regenerating it an edit rather than a copy; `rlike` is also not alone — `like`, `glob`, `and`, `or`, `xor` and the comparison names are in the same position. I have documented the caveat in the fixture header instead, along with the point that this file backs the weaker claim of "a name Databend knows", not "callable in that position".
Tests are 77 → 82. Thanks for the first one especially — it was a genuine silent-data-corruption path.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat(databend): add a sqlglot dialect based on Postgres [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43964:
URL: https://github.com/apache/superset/pull/43964#issuecomment-5636164889
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org