aminghadersohi commented on PR #43680: URL: https://github.com/apache/superset/pull/43680#issuecomment-5481471534
@richardfogaca — assigning you as reviewer here. **Why you:** you approved #38700 (the PR that introduced `get_chart_sql`) and you've been across the recent chart-data/query-object work (#43520, #43081), so this sits squarely in territory you already know. **Worth scrutinising:** - `_get_query` in `superset/common/query_actions.py` — the `getattr(..., "get_query_str_extended", None)` fallback, and the two new keys on the `query` result payload (also consumed by the "View query" modal, not just MCP). - `QueryStringExtended.full_sql` in `superset/models/helpers.py` — prequery-joining logic moved here from `ExploreMixin.get_query_str`; confirm the two definitions really were equivalent. - The new fail-closed guard in `get_chart_sql`: it's an intentional behavior change (previously returned unfiltered SQL as a success). The PR body calls it out explicitly. 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]
