dangdtr commented on PR #42474:
URL: https://github.com/apache/superset/pull/42474#issuecomment-5128417463
@rusackas Thanks for calling this out. You’re right that
`ChartDataQueryObjectSchema` lets a truthy `groupby` overwrite `columns`.
After tracing the exact path this optimization replaces, I found that
`Slice.get_query_context()` bypasses that Marshmallow schema and passes the
stored queries through `QueryContextFactory` to `QueryObjectFactory`. Since
#41204, the factory intentionally uses `setdefault`, so if both keys are
present, it keeps `columns` and ignores the deprecated `groupby` value. This is
also covered by `test_deprecated_groupby_does_not_overwrite_columns`.
I’ve updated the extractor to mirror that factory behavior and added
regression coverage for the both-keys case. This preserves the behavior of the
path being replaced while still avoiding per-slice `QueryContext` construction
and datasource lookups. I also hardened malformed column labels so they fall
back to `form_data` instead of raising `TypeError`. Fixed in `b08066c`. Thanks!
--
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]
