rusackas commented on PR #42474: URL: https://github.com/apache/superset/pull/42474#issuecomment-5127236263
Thanks for digging into the N+1 here, dangdtr, rebuilding a full QueryContext per slice just to read column names is real waste. One thing before I'd approve, though: `_extract_query_context_columns` prefers `columns` over `groupby` when both keys are present, but `ChartDataQueryObjectSchema`'s own rename logic does the opposite, `groupby` overwrites `columns` when truthy. For an old dashboard whose serialized query_context has both, this could quietly drop columns the original `get_query_context()` path would've picked up. Worth matching that precedence, or is that combo not actually possible in practice? -- 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]
