codeant-ai-for-open-source[bot] commented on code in PR #42474:
URL: https://github.com/apache/superset/pull/42474#discussion_r3657088040
##########
superset/connectors/sqla/models.py:
##########
@@ -526,28 +560,11 @@ def data_for_slices( # pylint: disable=too-many-locals
# noqa: C901
if "column" in filter_config
)
- # for legacy dashboard imports which have the wrong query_context
in them
- try:
- query_context = slc.get_query_context()
- except (DatasetNotFoundError, DatasourceNotFound):
- logger.warning(
- "Failed to load query_context for chart '%s' (id=%s): "
- "referenced datasource not found",
- slc.slice_name,
- slc.id,
- )
- query_context = None
-
- # legacy charts don't have query_context charts
- if query_context:
- column_names.update(
- [
- utils.get_column_name(column_)
- for query in query_context.queries
- for column_ in query.columns
- ]
- or []
- )
+ query_context_column_names = self._extract_query_context_columns(
+ slc.query_context
+ )
+ if query_context_column_names is not None:
+ column_names.update(query_context_column_names)
else:
_columns = [
Review Comment:
**Suggestion:** When `queries` is an empty list or contains only
invalid/non-dictionary entries, `_extract_query_context_columns` returns an
empty set rather than `None`. The caller treats that as a successfully parsed
query context and skips the form-data fallback, so required columns such as
`groupby` fields can be omitted from the serialized datasource. Return `None`
when no valid column entries were extracted so malformed or unusable query
contexts use the existing fallback. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard datasource payload omits columns for empty query contexts.
- ⚠️ Chart filters may lack required groupby or form-data columns.
- ⚠️ Malformed query contexts do not consistently use fallback behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load a dashboard through the dataset serialization path that calls
`SqlaTable.data_for_slices()` in `superset/connectors/sqla/models.py:525`,
passing a slice
whose `query_context` contains valid JSON with an empty `queries` list, such
as
`{"queries": []}`.
2. `data_for_slices()` collects form-data columns, including adhoc-filter
and filter-box
columns at `superset/connectors/sqla/models.py:550-561`, then calls
`_extract_query_context_columns()` at
`superset/connectors/sqla/models.py:563-565`.
3. `_extract_query_context_columns()` accepts the empty list at
`superset/connectors/sqla/models.py:505-507`, leaves `column_names` empty,
and returns
`set()` at `superset/connectors/sqla/models.py:523` rather than `None`.
4. The condition at `superset/connectors/sqla/models.py:566` therefore
succeeds, and
execution skips the fallback beginning at
`superset/connectors/sqla/models.py:568-574`, so
columns normally obtained from form-data query parameters are not added to
the datasource
payload.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5a92dc60a0254195a21f250fc46ec6a9&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=5a92dc60a0254195a21f250fc46ec6a9&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:** 566:569
**Comment:**
*Logic Error: When `queries` is an empty list or contains only
invalid/non-dictionary entries, `_extract_query_context_columns` returns an
empty set rather than `None`. The caller treats that as a successfully parsed
query context and skips the form-data fallback, so required columns such as
`groupby` fields can be omitted from the serialized datasource. Return `None`
when no valid column entries were extracted so malformed or unusable query
contexts use the existing fallback.
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=943b76d5e6f07fc7b4c465babc6c575002b927d94ff5a79edc1313b5ef25b27c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42474&comment_hash=943b76d5e6f07fc7b4c465babc6c575002b927d94ff5a79edc1313b5ef25b27c&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]
