codeant-ai-for-open-source[bot] commented on code in PR #38680:
URL: https://github.com/apache/superset/pull/38680#discussion_r2969656809
##########
superset/models/helpers.py:
##########
@@ -2735,6 +2735,20 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
m.metric_name: m for m in self.metrics
}
+ metric_names = {
+ m.metric_name
+ for m in self.metrics
+ if getattr(m, "metric_name", None)
+ }
+
+ for col in self.columns:
+ if (
+ col.verbose_name
+ and col.verbose_name not in columns_by_name
+ and col.verbose_name not in metric_names
+ ):
+ columns_by_name[col.verbose_name] = col
+
Review Comment:
**Suggestion:** The new verbose-name mapping only updates `columns_by_name`,
but `quoted_columns_by_name` is built earlier and never updated. In non-grouped
column queries, lookup happens against `quoted_columns_by_name`, so a verbose
label can be treated as a raw SQL literal instead of a real dataset column,
causing invalid SQL or wrong column resolution. Update the quoted map at the
same time you add verbose-name aliases. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Non-grouped verbose-column selects can resolve incorrect SQL
expressions.
- ⚠️ Query generation path becomes inconsistent across grouped versus
non-grouped.
```
</details>
```suggestion
quoted_columns_by_name[quote(col.verbose_name)] = col
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Trigger SQL generation through chart query flow
(`superset/charts/data/api.py:140` POST
`/api/v1/chart/data` or `:22` saved chart path), which builds a
`QueryContext` and
eventually calls datasource query execution.
2. Execution reaches `QueryContextProcessor.get_df_payload()`
(`superset/common/query_context_processor.py:62`) → `get_query_result()`
(`:166-174`) →
datasource `get_sqla_query()` in `superset/models/helpers.py`.
3. In `get_sqla_query()`, verbose aliases are added only to `columns_by_name`
(`helpers.py:2744-2750`), but `quoted_columns_by_name` was already frozen
earlier
(`helpers.py:2732`).
4. For non-grouped column selects (`need_groupby` false at
`helpers.py:2722`; branch at
`helpers.py:2905+`), lookup uses `quoted_columns_by_name`
(`helpers.py:2923-2928`). A
verbose-name column misses that map and falls back to
`literal_column(selected)`
(`helpers.py:2929-2930`), producing raw SQL literal handling instead of
dataset-column
resolution.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 2751:2751
**Comment:**
*Logic Error: The new verbose-name mapping only updates
`columns_by_name`, but `quoted_columns_by_name` is built earlier and never
updated. In non-grouped column queries, lookup happens against
`quoted_columns_by_name`, so a verbose label can be treated as a raw SQL
literal instead of a real dataset column, causing invalid SQL or wrong column
resolution. Update the quoted map at the same time you add verbose-name aliases.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38680&comment_hash=bf53d830f26de8f6d42369b0c40a907c46c85c4b8103c6174deba4aac8fc9616&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38680&comment_hash=bf53d830f26de8f6d42369b0c40a907c46c85c4b8103c6174deba4aac8fc9616&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]
