codeant-ai-for-open-source[bot] commented on code in PR #42298:
URL: https://github.com/apache/superset/pull/42298#discussion_r3627648345
##########
superset/db_engine_specs/clickhouse.py:
##########
@@ -531,3 +531,15 @@ def adjust_engine_params(
if schema:
uri = uri.set(database=parse.quote(schema, safe=""))
return uri, connect_args
+
+ @classmethod
+ def get_column_description_retry_sql(cls, sql: str) -> str | None:
+ # clickhouse-connect's cursor only backfills `cursor.description` for
+ # a zero-row result -- e.g. the `WHERE false` probe used to detect an
+ # adhoc column's type without scanning any rows -- when the operation
+ # string starts with SELECT/WITH after stripping whitespace. Leading
+ # SQL comments inserted by SQL_QUERY_MUTATOR (e.g. query attribution)
+ # defeat that check, so wrap the untouched, already-mutated SQL in a
+ # bare outer SELECT to satisfy it without altering or dropping any of
+ # the mutator's comments.
+ return f"SELECT * FROM (\n{sql}\n) AS __superset_type_probe LIMIT 0"
# noqa: S608
Review Comment:
**Suggestion:** The retry wrapper embeds `sql` directly as a subquery
without normalizing statement terminators. If the mutated SQL ends with a
semicolon (a common mutator/output pattern), the generated `FROM (...)`
subquery becomes invalid in ClickHouse and the retry will fail with a SQL parse
error instead of recovering metadata. Strip a trailing semicolon (and
surrounding whitespace) before wrapping. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ ClickHouse adhoc expression charts fail with mutator semicolons.
- ⚠️ Retry metadata probe breaks, leaving columns undiscoverable.
- ⚠️ Error handling path introduces new ClickHouse parse failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure `SQL_QUERY_MUTATOR` in `superset/config.py` to append a
trailing semicolon to
all queries it mutates, then deploy Superset with ClickHouse as the database
backend. This
matches the description’s supported customization point and causes the probe
SQL passed
into `ClickHouseEngineSpec.get_column_description_retry_sql()` at
`superset/db_engine_specs/clickhouse.py:535-545` to end with `;`.
2. Create or open a chart that uses an adhoc SQL-expression column on
ClickHouse,
triggering the zero-row type probe described in the PR (the probe query is
built with
`WHERE false`, then sent through `SQL_QUERY_MUTATOR`, and finally into
`get_columns_description()` which calls `get_column_description_retry_sql()`
when the
initial ClickHouse metadata lookup returns no columns).
3. When the retry path runs, `get_column_description_retry_sql()` at
`superset/db_engine_specs/clickhouse.py:536-545` wraps the already-mutated
SQL, which now
ends with `;`, as:
`SELECT * FROM (\n<original_sql_with_semicolon>\n) AS
__superset_type_probe LIMIT 0`
and sends this composite query to ClickHouse via clickhouse-connect.
4. In ClickHouse, the inner subquery `(<original_sql_with_semicolon>)` is
parsed as
invalid because a semicolon terminator is not allowed inside the subquery
parentheses;
clickhouse-connect surfaces a SQL parse error, the retry never recovers
`cursor.description`, and the adhoc column type resolution fails, causing
the chart query
to error instead of loading.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b408d2b4ed344b788554788fdfc66858&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=b408d2b4ed344b788554788fdfc66858&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/clickhouse.py
**Line:** 545:545
**Comment:**
*Logic Error: The retry wrapper embeds `sql` directly as a subquery
without normalizing statement terminators. If the mutated SQL ends with a
semicolon (a common mutator/output pattern), the generated `FROM (...)`
subquery becomes invalid in ClickHouse and the retry will fail with a SQL parse
error instead of recovering metadata. Strip a trailing semicolon (and
surrounding whitespace) before wrapping.
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%2F42298&comment_hash=9b84b1d4861317e722d256d171c0f6d794fb4f33e1fc50c0c5a1b18c069c6a26&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42298&comment_hash=9b84b1d4861317e722d256d171c0f6d794fb4f33e1fc50c0c5a1b18c069c6a26&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: [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]
