codeant-ai-for-open-source[bot] commented on code in PR #42474:
URL: https://github.com/apache/superset/pull/42474#discussion_r3680990021
##########
superset/connectors/sqla/models.py:
##########
@@ -488,6 +486,42 @@ def data(self) -> ExplorableData:
"select_star": self.select_star,
}
+ @staticmethod
+ def _extract_query_context_columns(query_context: str | None) -> set[str]
| None:
+ """Extract column names from a serialized query context."""
+ if not query_context:
+ return None
+
+ try:
+ payload = json.loads(query_context)
+ except json.JSONDecodeError as ex:
+ logger.error("Malformed json in slice's query context",
exc_info=True)
+ logger.exception(ex)
+ return None
+
+ if not isinstance(payload, dict):
+ return None
+
+ queries = payload.get("queries")
+ if not isinstance(queries, list):
+ return None
+
+ column_names: set[str] = set()
+ for query in queries:
+ if not isinstance(query, dict):
+ continue
+ columns = query.get("groupby") or query.get("columns")
+ if not isinstance(columns, list):
+ continue
+ try:
+ column_names.update(
+ utils.get_column_name(column_) for column_ in columns
+ )
Review Comment:
**Suggestion:** `get_column_name` can return a non-string, unhashable value
when a malformed JSON column has a list or dictionary as its `label`.
`set.update` then raises `TypeError`, which is not handled, causing dashboard
datasource serialization to fail instead of using the documented form-data
fallback. Validate the extracted name or catch this malformed-column case.
[type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard dataset serialization raises an uncaught TypeError.
- ❌ Dashboard loading can fail for malformed chart metadata.
- ⚠️ Form-data fallback is bypassed for invalid labels.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c944b901693d415d8a31d649a4a3cfe2&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=c944b901693d415d8a31d649a4a3cfe2&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/connectors/sqla/models.py
**Line:** 517:519
**Comment:**
*Type Error: `get_column_name` can return a non-string, unhashable
value when a malformed JSON column has a list or dictionary as its `label`.
`set.update` then raises `TypeError`, which is not handled, causing dashboard
datasource serialization to fail instead of using the documented form-data
fallback. Validate the extracted name or catch this malformed-column case.
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%2F42474&comment_hash=39bd3a4c69fa4ea80c7882230af8970255a57f2f8742bcaf4c54f2ed8e3ff14f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42474&comment_hash=39bd3a4c69fa4ea80c7882230af8970255a57f2f8742bcaf4c54f2ed8e3ff14f&reaction=dislike'>👎</a>
##########
superset/connectors/sqla/models.py:
##########
@@ -488,6 +486,42 @@ def data(self) -> ExplorableData:
"select_star": self.select_star,
}
+ @staticmethod
+ def _extract_query_context_columns(query_context: str | None) -> set[str]
| None:
+ """Extract column names from a serialized query context."""
+ if not query_context:
+ return None
+
+ try:
+ payload = json.loads(query_context)
+ except json.JSONDecodeError as ex:
+ logger.error("Malformed json in slice's query context",
exc_info=True)
+ logger.exception(ex)
+ return None
+
+ if not isinstance(payload, dict):
+ return None
+
+ queries = payload.get("queries")
+ if not isinstance(queries, list):
+ return None
+
+ column_names: set[str] = set()
+ for query in queries:
+ if not isinstance(query, dict):
+ continue
+ columns = query.get("groupby") or query.get("columns")
Review Comment:
**Suggestion:** This precedence differs from `QueryObjectFactory`, which
renames `groupby` to `columns` only when the canonical `columns` field is
absent. When both fields are present, the normalized query uses `columns`, but
this code chooses a truthy deprecated `groupby` value and ignores the canonical
columns. The extracted datasource metadata can therefore omit columns that the
query actually uses; prefer `columns` whenever it is present and valid, falling
back to `groupby` only when it is absent. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Serialized datasource metadata can omit executed columns.
- ⚠️ Dashboard chart rendering receives incomplete column metadata.
- ⚠️ Mixed-version query contexts produce inconsistent behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f69e07051fcb4cd9a44bf9e9f21b9a3b&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=f69e07051fcb4cd9a44bf9e9f21b9a3b&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/connectors/sqla/models.py
**Line:** 513:513
**Comment:**
*Api Mismatch: This precedence differs from `QueryObjectFactory`, which
renames `groupby` to `columns` only when the canonical `columns` field is
absent. When both fields are present, the normalized query uses `columns`, but
this code chooses a truthy deprecated `groupby` value and ignores the canonical
columns. The extracted datasource metadata can therefore omit columns that the
query actually uses; prefer `columns` whenever it is present and valid, falling
back to `groupby` only when it is absent.
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%2F42474&comment_hash=734d88fc6c374471c5520393018d7eea96e71797e18d9cfde67b7f344b59a9fa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42474&comment_hash=734d88fc6c374471c5520393018d7eea96e71797e18d9cfde67b7f344b59a9fa&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]
