codeant-ai-for-open-source[bot] commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r3481285689
##########
superset/utils/core.py:
##########
@@ -1852,6 +1852,42 @@ def extract_dataframe_dtypes(
return generic_types
+def extract_display_labels(
+ label_map: dict[str, list[str]],
+ colnames: list[str],
+ datasource: Explorable | None = None,
+) -> list[str]:
+ """Extract display labels for a list of column names based on a label map
+ and an optional datasource.
+ """
+ if not colnames:
+ return []
+
+ # Build column -> label mapping (skip self-references)
+ columns_to_label = {}
+ if label_map:
+ for label, cols in label_map.items():
+ for col in cols:
+ if label != col and col not in columns_to_label:
+ columns_to_label[col] = label
+
+ # Build column -> object mapping
+ columns_by_name: dict[str, Any] = {}
+ if datasource:
+ for column in datasource.columns:
+ if isinstance(column, dict):
+ if column_name := column.get("column_name"):
+ columns_by_name[column_name] = column
+ else:
+ columns_by_name[column.column_name] = column
+
+ return [
+ columns_to_label.get(col)
+ or (get_column_name(columns_by_name[col]) if col in columns_by_name
else col)
+ for col in colnames
+ ]
Review Comment:
**Suggestion:** `extract_display_labels` can raise a runtime `ValueError`
when `datasource.columns` contains dict-based column metadata (a supported
shape in this codebase) because it passes that dict into `get_column_name`,
which only accepts dicts with `label`/`sqlExpression` and will fail for common
`{column_name, verbose_name}` entries. Handle dict columns explicitly here
(prefer `verbose_name`, then `column_name`) instead of calling
`get_column_name` on the raw dict. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Query results payload fails when dict columns used.
- ⚠️ Explore UI table headers may not render successfully.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Execute a chart query through `QueryContextProcessor.get_df_payload` in
`superset/common/query_context_processor.py:82-226`, so that a
`QueryContext` is created
with its `datasource` implementing the `Explorable` protocol.
2. Use an `Explorable` implementation whose `columns` list contains
dict-based column
metadata shaped like `DatasetColumnData` (see
`superset/superset_typing.py:9-29`) rather
than ORM objects; this is the same structure produced by
`BaseDatasource.data` at
`superset/connectors/sqla/models.py:36-37` where `"columns":
[cast(DatasetColumnData,
o.data) for o in self.columns]`.
3. When `_get_full` in `superset/common/query_actions.py:154-176` runs for
the query, it
calls `extract_display_labels(payload["label_map"], payload["colnames"],
datasource)` to
populate `payload["collabels"]` for the results payload.
4. Inside `extract_display_labels` (`superset/utils/core.py:1855-1888`),
each dict column
is stored in `columns_by_name`; for a column name `col` with no entry in
`columns_to_label`, the code evaluates
`get_column_name(columns_by_name[col])`. Since
`columns_by_name[col]` is a dict with `column_name`/`verbose_name` only,
`get_column_name`
(`superset/utils/core.py:7-32`) falls through its dict and string cases and
raises
`ValueError("Missing label")`, causing the query results payload
construction to fail at
runtime.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2f877fc93b69460e888fd911ab7bd460&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=2f877fc93b69460e888fd911ab7bd460&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/utils/core.py
**Line:** 1884:1888
**Comment:**
*Type Error: `extract_display_labels` can raise a runtime `ValueError`
when `datasource.columns` contains dict-based column metadata (a supported
shape in this codebase) because it passes that dict into `get_column_name`,
which only accepts dicts with `label`/`sqlExpression` and will fail for common
`{column_name, verbose_name}` entries. Handle dict columns explicitly here
(prefer `verbose_name`, then `column_name`) instead of calling
`get_column_name` on the raw dict.
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%2F37396&comment_hash=3ad41b77c3ee9a8113fb63c4c53416bee79927f014656863b1257b38034a5276&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37396&comment_hash=3ad41b77c3ee9a8113fb63c4c53416bee79927f014656863b1257b38034a5276&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]
