codeant-ai-for-open-source[bot] commented on code in PR #42284: URL: https://github.com/apache/superset/pull/42284#discussion_r3675696326
########## superset/common/form_data_query_context.py: ########## @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Synthesize a query context from a chart's saved form data (``params``). + +A chart's ``query_context`` is normally generated client-side by each viz +plugin's ``buildQuery`` and only persisted when the chart is (re-)saved in +Explore. Charts that predate that behavior keep their ``params`` (form data) but +carry no ``query_context``, so server-side consumers that need to run the query +(e.g. the dashboard Excel export) have nothing to execute. + +This module rebuilds a best-effort query context from the form data — columns, +metrics, filters (including free-form SQL and the time range), ordering and time +grain — mirroring the shared parts of the viz plugins' ``buildQuery``. It does +**not** reproduce plugin post-processing (pivot, contribution/percent +transforms, rolling/forecast) or multi-query fan-out, so callers must restrict it +to viz types whose data maps faithfully to a single plain query. +""" + +from __future__ import annotations + +from typing import Any + +from superset.utils import json + + +def adhoc_filters_to_query_filters( + adhoc_filters: list[dict[str, Any]], + where_only: bool = False, +) -> list[dict[str, Any]]: + """ + Convert ``SIMPLE`` adhoc filters into QueryObject filter clauses. + + Adhoc filters use ``{subject, operator, comparator}`` while a query object + expects ``{col, op, val}``; free-form ``SQL`` filters have no ``{col, op, + val}`` equivalent and are handled separately (see + :func:`freeform_where_having`). + + By default all ``SIMPLE`` filters are converted (the behavior the MCP + compile/preview path relies on). Pass ``where_only=True`` to convert only + ``WHERE``-clause filters, matching the frontend's ``processFilters`` — the + dashboard export uses this so it applies the same rows the chart shows and + does not additionally filter on ``SIMPLE`` ``HAVING`` clauses. + """ + result: list[dict[str, Any]] = [] + for flt in adhoc_filters or []: + if flt.get("expressionType") != "SIMPLE": + continue + if where_only and (flt.get("clause") or "WHERE").upper() != "WHERE": + continue + result.append( + { + "col": flt.get("subject"), + "op": flt.get("operator"), + "val": flt.get("comparator"), + } + ) + return result + + +def freeform_where_having(form_data: dict[str, Any]) -> dict[str, str]: + """ + Collect free-form SQL predicates into a query ``extras`` mapping. + + Mirrors ``processFilters`` on the frontend: ``SQL`` adhoc filters (and a + legacy top-level ``where``) join into ``extras.where`` / ``extras.having`` by + clause, so a chart restricted by a custom SQL predicate exports the same rows + it displays instead of the full, unrestricted result. + """ + where: list[str] = [] + having: list[str] = [] + if form_data.get("where"): + where.append(form_data["where"]) + for flt in form_data.get("adhoc_filters") or []: + if flt.get("expressionType") == "SQL" and flt.get("sqlExpression"): + clause = (flt.get("clause") or "WHERE").upper() + (having if clause == "HAVING" else where).append(flt["sqlExpression"]) + + extras: dict[str, str] = {} + if where: + extras["where"] = " AND ".join(f"({clause})" for clause in where) + if having: + extras["having"] = " AND ".join(f"({clause})" for clause in having) + return extras + + +def columns_from_form_data(form_data: dict[str, Any]) -> list[Any]: + """ + Derive the query's grouping/raw columns from form data. + + Handles raw-mode tables (``all_columns``/``columns``), an ``x_axis`` (string + or adhoc column), and ``groupby`` dimensions, de-duplicating while preserving + order. + """ + if form_data.get("query_mode") == "raw" and ( + form_data.get("all_columns") or form_data.get("columns") + ): + return list(form_data.get("all_columns") or form_data.get("columns") or []) + + groupby_columns: list[Any] = form_data.get("groupby") or [] + raw_columns: list[Any] = form_data.get("columns") or [] + # Prefer explicit raw columns only when they are actually present; a stale + # empty ``columns: []`` key must not shadow the group-by dimensions (which + # would silently drop the grouping and change the aggregation). + columns = raw_columns.copy() if raw_columns else groupby_columns.copy() + + x_axis = form_data.get("x_axis") + if isinstance(x_axis, str) and x_axis and x_axis not in columns: + columns.insert(0, x_axis) + elif isinstance(x_axis, dict): + col_name = x_axis.get("column_name") + if col_name and col_name not in columns: + columns.insert(0, col_name) + return columns + + +def is_raw_query_mode(form_data: dict[str, Any]) -> bool: + """ + Whether the chart runs in raw (non-aggregated) mode, mirroring the frontend's + ``getQueryMode``: an explicit ``query_mode`` wins, otherwise the presence of + ``all_columns`` implies raw mode. + """ + if mode := form_data.get("query_mode"): + return mode == "raw" + return bool(form_data.get("all_columns")) + + +def orderby_from_form_data( + form_data: dict[str, Any], metrics: list[Any], viz_type: str | None = None +) -> list[list[Any]]: + """ + Derive ordering so a ``row_limit`` returns the chart's top-N, not an + arbitrary N. + + Raw-mode tables order by ``order_by_cols`` (stored as JSON ``[col, asc]`` + pairs). Aggregate charts order by the configured sort metric + (``timeseries_limit_metric``, or the first metric when ``sort_by_metric`` is + set), otherwise fall back to the first metric descending — matching the + table/pie ``buildQuery`` defaults. + """ + if order_by_cols := form_data.get("order_by_cols") or []: + parsed: list[list[Any]] = [] + for col in order_by_cols: + if isinstance(col, str): + try: + col = json.loads(col) + except (TypeError, ValueError): + continue + parsed.append(col) + return parsed Review Comment: **Suggestion:** Successfully parsed `order_by_cols` values are appended without validating that they are two-element `[column, ascending]` pairs. Values such as `null`, a scalar, a dictionary, or a one-element list reach `ChartDataQueryContextSchema`, which expects tuple pairs and rejects the entire chart export instead of treating the malformed legacy ordering as absent. Validate the shape before appending it. [type error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Malformed ordering can abort chart Excel export. - ⚠️ One legacy chart can be skipped from the workbook. - ⚠️ Recoverable form-data corruption becomes a user-visible error. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=13cbb7dbd4fd4dfd9cbb9e0807405654&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=13cbb7dbd4fd4dfd9cbb9e0807405654&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/common/form_data_query_context.py **Line:** 155:164 **Comment:** *Type Error: Successfully parsed `order_by_cols` values are appended without validating that they are two-element `[column, ascending]` pairs. Values such as `null`, a scalar, a dictionary, or a one-element list reach `ChartDataQueryContextSchema`, which expects tuple pairs and rejects the entire chart export instead of treating the malformed legacy ordering as absent. Validate the shape before appending it. 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%2F42284&comment_hash=1d6de6cec9830db57aa412c1d022de22ece118f8b038071da9eb26bc07b2733f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=1d6de6cec9830db57aa412c1d022de22ece118f8b038071da9eb26bc07b2733f&reaction=dislike'>👎</a> ########## superset/common/form_data_query_context.py: ########## @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Synthesize a query context from a chart's saved form data (``params``). + +A chart's ``query_context`` is normally generated client-side by each viz +plugin's ``buildQuery`` and only persisted when the chart is (re-)saved in +Explore. Charts that predate that behavior keep their ``params`` (form data) but +carry no ``query_context``, so server-side consumers that need to run the query +(e.g. the dashboard Excel export) have nothing to execute. + +This module rebuilds a best-effort query context from the form data — columns, +metrics, filters (including free-form SQL and the time range), ordering and time +grain — mirroring the shared parts of the viz plugins' ``buildQuery``. It does +**not** reproduce plugin post-processing (pivot, contribution/percent +transforms, rolling/forecast) or multi-query fan-out, so callers must restrict it +to viz types whose data maps faithfully to a single plain query. +""" + +from __future__ import annotations + +from typing import Any + +from superset.utils import json + + +def adhoc_filters_to_query_filters( + adhoc_filters: list[dict[str, Any]], + where_only: bool = False, +) -> list[dict[str, Any]]: + """ + Convert ``SIMPLE`` adhoc filters into QueryObject filter clauses. + + Adhoc filters use ``{subject, operator, comparator}`` while a query object + expects ``{col, op, val}``; free-form ``SQL`` filters have no ``{col, op, + val}`` equivalent and are handled separately (see + :func:`freeform_where_having`). + + By default all ``SIMPLE`` filters are converted (the behavior the MCP + compile/preview path relies on). Pass ``where_only=True`` to convert only + ``WHERE``-clause filters, matching the frontend's ``processFilters`` — the + dashboard export uses this so it applies the same rows the chart shows and + does not additionally filter on ``SIMPLE`` ``HAVING`` clauses. + """ + result: list[dict[str, Any]] = [] + for flt in adhoc_filters or []: + if flt.get("expressionType") != "SIMPLE": + continue + if where_only and (flt.get("clause") or "WHERE").upper() != "WHERE": + continue + result.append( + { + "col": flt.get("subject"), + "op": flt.get("operator"), + "val": flt.get("comparator"), + } + ) + return result + + +def freeform_where_having(form_data: dict[str, Any]) -> dict[str, str]: + """ + Collect free-form SQL predicates into a query ``extras`` mapping. + + Mirrors ``processFilters`` on the frontend: ``SQL`` adhoc filters (and a + legacy top-level ``where``) join into ``extras.where`` / ``extras.having`` by + clause, so a chart restricted by a custom SQL predicate exports the same rows + it displays instead of the full, unrestricted result. + """ + where: list[str] = [] + having: list[str] = [] + if form_data.get("where"): + where.append(form_data["where"]) + for flt in form_data.get("adhoc_filters") or []: + if flt.get("expressionType") == "SQL" and flt.get("sqlExpression"): + clause = (flt.get("clause") or "WHERE").upper() + (having if clause == "HAVING" else where).append(flt["sqlExpression"]) Review Comment: **Suggestion:** Unlike the frontend filter processor, this path does not append a newline when a free-form SQL clause contains `--`. The generated wrapper can therefore turn the closing parenthesis and following SQL into a comment, causing a syntax error or changing the predicate semantics during export. Apply the same clause sanitization as `processFilters` before placing expressions in `extras`. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ SQL-filtered Excel exports can fail query execution. - ⚠️ Export predicates can differ from displayed chart results. - ⚠️ Affected charts are omitted or reported as export errors. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a04831f9e8cb4c7885324b90dc2dc267&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=a04831f9e8cb4c7885324b90dc2dc267&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/common/form_data_query_context.py **Line:** 86:91 **Comment:** *Logic Error: Unlike the frontend filter processor, this path does not append a newline when a free-form SQL clause contains `--`. The generated wrapper can therefore turn the closing parenthesis and following SQL into a comment, causing a syntax error or changing the predicate semantics during export. Apply the same clause sanitization as `processFilters` before placing expressions in `extras`. 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%2F42284&comment_hash=2dbd4d39c8ce1dc5e7ca21a07c207dc796fa8799a78ed5384a6a202412a45e16&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=2dbd4d39c8ce1dc5e7ca21a07c207dc796fa8799a78ed5384a6a202412a45e16&reaction=dislike'>👎</a> ########## superset/common/form_data_query_context.py: ########## @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Synthesize a query context from a chart's saved form data (``params``). + +A chart's ``query_context`` is normally generated client-side by each viz +plugin's ``buildQuery`` and only persisted when the chart is (re-)saved in +Explore. Charts that predate that behavior keep their ``params`` (form data) but +carry no ``query_context``, so server-side consumers that need to run the query +(e.g. the dashboard Excel export) have nothing to execute. + +This module rebuilds a best-effort query context from the form data — columns, +metrics, filters (including free-form SQL and the time range), ordering and time +grain — mirroring the shared parts of the viz plugins' ``buildQuery``. It does +**not** reproduce plugin post-processing (pivot, contribution/percent +transforms, rolling/forecast) or multi-query fan-out, so callers must restrict it +to viz types whose data maps faithfully to a single plain query. +""" + +from __future__ import annotations + +from typing import Any + +from superset.utils import json + + +def adhoc_filters_to_query_filters( + adhoc_filters: list[dict[str, Any]], + where_only: bool = False, +) -> list[dict[str, Any]]: + """ + Convert ``SIMPLE`` adhoc filters into QueryObject filter clauses. + + Adhoc filters use ``{subject, operator, comparator}`` while a query object + expects ``{col, op, val}``; free-form ``SQL`` filters have no ``{col, op, + val}`` equivalent and are handled separately (see + :func:`freeform_where_having`). + + By default all ``SIMPLE`` filters are converted (the behavior the MCP + compile/preview path relies on). Pass ``where_only=True`` to convert only + ``WHERE``-clause filters, matching the frontend's ``processFilters`` — the + dashboard export uses this so it applies the same rows the chart shows and + does not additionally filter on ``SIMPLE`` ``HAVING`` clauses. + """ + result: list[dict[str, Any]] = [] + for flt in adhoc_filters or []: + if flt.get("expressionType") != "SIMPLE": + continue + if where_only and (flt.get("clause") or "WHERE").upper() != "WHERE": + continue + result.append( + { + "col": flt.get("subject"), + "op": flt.get("operator"), + "val": flt.get("comparator"), + } + ) + return result + + +def freeform_where_having(form_data: dict[str, Any]) -> dict[str, str]: + """ + Collect free-form SQL predicates into a query ``extras`` mapping. + + Mirrors ``processFilters`` on the frontend: ``SQL`` adhoc filters (and a + legacy top-level ``where``) join into ``extras.where`` / ``extras.having`` by + clause, so a chart restricted by a custom SQL predicate exports the same rows + it displays instead of the full, unrestricted result. + """ + where: list[str] = [] + having: list[str] = [] + if form_data.get("where"): + where.append(form_data["where"]) + for flt in form_data.get("adhoc_filters") or []: + if flt.get("expressionType") == "SQL" and flt.get("sqlExpression"): + clause = (flt.get("clause") or "WHERE").upper() + (having if clause == "HAVING" else where).append(flt["sqlExpression"]) + + extras: dict[str, str] = {} + if where: + extras["where"] = " AND ".join(f"({clause})" for clause in where) + if having: + extras["having"] = " AND ".join(f"({clause})" for clause in having) + return extras + + +def columns_from_form_data(form_data: dict[str, Any]) -> list[Any]: + """ + Derive the query's grouping/raw columns from form data. + + Handles raw-mode tables (``all_columns``/``columns``), an ``x_axis`` (string + or adhoc column), and ``groupby`` dimensions, de-duplicating while preserving + order. + """ + if form_data.get("query_mode") == "raw" and ( + form_data.get("all_columns") or form_data.get("columns") + ): + return list(form_data.get("all_columns") or form_data.get("columns") or []) + + groupby_columns: list[Any] = form_data.get("groupby") or [] + raw_columns: list[Any] = form_data.get("columns") or [] + # Prefer explicit raw columns only when they are actually present; a stale + # empty ``columns: []`` key must not shadow the group-by dimensions (which + # would silently drop the grouping and change the aggregation). + columns = raw_columns.copy() if raw_columns else groupby_columns.copy() + + x_axis = form_data.get("x_axis") + if isinstance(x_axis, str) and x_axis and x_axis not in columns: + columns.insert(0, x_axis) + elif isinstance(x_axis, dict): + col_name = x_axis.get("column_name") + if col_name and col_name not in columns: + columns.insert(0, col_name) + return columns + + +def is_raw_query_mode(form_data: dict[str, Any]) -> bool: + """ + Whether the chart runs in raw (non-aggregated) mode, mirroring the frontend's + ``getQueryMode``: an explicit ``query_mode`` wins, otherwise the presence of + ``all_columns`` implies raw mode. + """ + if mode := form_data.get("query_mode"): + return mode == "raw" + return bool(form_data.get("all_columns")) + + +def orderby_from_form_data( + form_data: dict[str, Any], metrics: list[Any], viz_type: str | None = None +) -> list[list[Any]]: + """ + Derive ordering so a ``row_limit`` returns the chart's top-N, not an + arbitrary N. + + Raw-mode tables order by ``order_by_cols`` (stored as JSON ``[col, asc]`` + pairs). Aggregate charts order by the configured sort metric + (``timeseries_limit_metric``, or the first metric when ``sort_by_metric`` is + set), otherwise fall back to the first metric descending — matching the + table/pie ``buildQuery`` defaults. + """ + if order_by_cols := form_data.get("order_by_cols") or []: + parsed: list[list[Any]] = [] + for col in order_by_cols: + if isinstance(col, str): + try: + col = json.loads(col) + except (TypeError, ValueError): + continue + parsed.append(col) + return parsed + + if not metrics: + return [] + + sort_metric = form_data.get("timeseries_limit_metric") or ( + metrics[0] if form_data.get("sort_by_metric") else None + ) + if sort_metric is not None: + # The Table plugin defaults ``order_desc`` to False (ascending); Pie and + # others sort by metric descending. Match that so a row limit keeps the + # chart's top/bottom-N rather than flipping it. + default_desc = viz_type != "table" + order_desc = form_data.get("order_desc", default_desc) + return [[sort_metric, not order_desc]] + # No explicit sort metric: default to the first metric, descending. + return [[metrics[0], False]] + + +def _columns_and_metrics( + form_data: dict[str, Any], viz_type: str | None +) -> tuple[list[Any], list[Any], bool]: + """ + Resolve the query's ``(columns, metrics, promoted_time_column)`` from form + data, honoring raw vs. aggregate mode and the Big Number trendline promotion. + """ + if is_raw_query_mode(form_data): + # Raw mode returns individual rows: use only the selected columns and + # ignore ``metrics``/``groupby``, which stay in form data as stale values + # (the controls aren't reset when hidden) but are ignored by the chart. + columns = list(form_data.get("all_columns") or form_data.get("columns") or []) + return columns, [], False + + metrics = list(form_data.get("metrics") or []) + # Single-metric charts (e.g. Big Number) store ``metric`` rather than + # ``metrics``. + if not metrics and form_data.get("metric"): + metrics = [form_data["metric"]] + columns = columns_from_form_data(form_data) + # Only a Big Number *with a trendline* (viz_type ``big_number``) groups by its + # time column; ``big_number_total`` is a single aggregate and must not be + # grouped, or it would return one row per timestamp instead of a total. + if not columns and viz_type == "big_number" and form_data.get("granularity_sqla"): + return [form_data["granularity_sqla"]], metrics, True + return columns, metrics, False + + +def build_query_context_from_form_data( + form_data: dict[str, Any], + datasource: dict[str, Any], + viz_type: str | None = None, +) -> dict[str, Any]: + """ + Build a query-context payload (the JSON shape ``ChartDataQueryContextSchema`` + loads) from a chart's form data and datasource reference. + + :param form_data: The chart's saved ``params`` parsed to a dict. + :param datasource: ``{"id": <int>, "type": "table"}`` datasource reference. + :param viz_type: The chart's viz type, used for viz-specific handling. + :returns: A single-query query-context dict. + """ + columns, metrics, promoted_time_column = _columns_and_metrics(form_data, viz_type) + + # SIMPLE adhoc filters (+ legacy top-level ``filters``) become query filters; + # free-form SQL predicates go into ``extras``. Only ``WHERE``-clause SIMPLE + # filters are applied (matching the chart), so the export never filters on a + # ``HAVING`` clause the chart itself ignores. + filters = adhoc_filters_to_query_filters( + form_data.get("adhoc_filters", []), where_only=True + ) + for flt in form_data.get("filters") or []: + if isinstance(flt, dict) and flt.get("col") is not None: + filters.append(flt) + + extras = freeform_where_having(form_data) + if form_data.get("time_grain_sqla"): + extras["time_grain_sqla"] = form_data["time_grain_sqla"] + + # Prefer the modern ``time_range``; fall back to the legacy ``since``/``until`` + # pair (older charts store the range that way) before defaulting to no filter. + time_range = form_data.get("time_range") + if not time_range and (form_data.get("since") or form_data.get("until")): + time_range = f"{form_data.get('since') or ''} : {form_data.get('until') or ''}" + time_range = time_range or "No filter" + query: dict[str, Any] = { + "columns": columns, + "metrics": metrics, + "orderby": orderby_from_form_data(form_data, metrics, viz_type), + "filters": filters, + "time_range": time_range, + } Review Comment: **Suggestion:** The generic rebuild omits table and other chart controls such as `series_limit` and `series_limit_metric`. A chart with a series limit can therefore return arbitrary groups up to `row_limit` instead of the configured top series, producing materially different export data while still being treated as a successful rebuild. These controls must be translated into the query context or such charts must be skipped. [api mismatch] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Allowlisted chart exports can contain incorrect groups. - ⚠️ Top-series and row-limit behavior diverges from charts. - ⚠️ Exported Excel data may not match displayed chart data. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=988401a3ca0f4f0bb641f1bc0431766b&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=988401a3ca0f4f0bb641f1bc0431766b&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/common/form_data_query_context.py **Line:** 248:254 **Comment:** *Api Mismatch: The generic rebuild omits table and other chart controls such as `series_limit` and `series_limit_metric`. A chart with a series limit can therefore return arbitrary groups up to `row_limit` instead of the configured top series, producing materially different export data while still being treated as a successful rebuild. These controls must be translated into the query context or such charts must be skipped. 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%2F42284&comment_hash=5986776cc8aca277243c3bb72482375394026f653e5c055d5987d35ea959598a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=5986776cc8aca277243c3bb72482375394026f653e5c055d5987d35ea959598a&reaction=dislike'>👎</a> ########## superset/common/form_data_query_context.py: ########## @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Synthesize a query context from a chart's saved form data (``params``). + +A chart's ``query_context`` is normally generated client-side by each viz +plugin's ``buildQuery`` and only persisted when the chart is (re-)saved in +Explore. Charts that predate that behavior keep their ``params`` (form data) but +carry no ``query_context``, so server-side consumers that need to run the query +(e.g. the dashboard Excel export) have nothing to execute. + +This module rebuilds a best-effort query context from the form data — columns, +metrics, filters (including free-form SQL and the time range), ordering and time +grain — mirroring the shared parts of the viz plugins' ``buildQuery``. It does +**not** reproduce plugin post-processing (pivot, contribution/percent +transforms, rolling/forecast) or multi-query fan-out, so callers must restrict it +to viz types whose data maps faithfully to a single plain query. +""" + +from __future__ import annotations + +from typing import Any + +from superset.utils import json + + +def adhoc_filters_to_query_filters( + adhoc_filters: list[dict[str, Any]], + where_only: bool = False, +) -> list[dict[str, Any]]: + """ + Convert ``SIMPLE`` adhoc filters into QueryObject filter clauses. + + Adhoc filters use ``{subject, operator, comparator}`` while a query object + expects ``{col, op, val}``; free-form ``SQL`` filters have no ``{col, op, + val}`` equivalent and are handled separately (see + :func:`freeform_where_having`). + + By default all ``SIMPLE`` filters are converted (the behavior the MCP + compile/preview path relies on). Pass ``where_only=True`` to convert only + ``WHERE``-clause filters, matching the frontend's ``processFilters`` — the + dashboard export uses this so it applies the same rows the chart shows and + does not additionally filter on ``SIMPLE`` ``HAVING`` clauses. + """ + result: list[dict[str, Any]] = [] + for flt in adhoc_filters or []: + if flt.get("expressionType") != "SIMPLE": + continue + if where_only and (flt.get("clause") or "WHERE").upper() != "WHERE": + continue + result.append( + { + "col": flt.get("subject"), + "op": flt.get("operator"), + "val": flt.get("comparator"), + } + ) + return result + + +def freeform_where_having(form_data: dict[str, Any]) -> dict[str, str]: + """ + Collect free-form SQL predicates into a query ``extras`` mapping. + + Mirrors ``processFilters`` on the frontend: ``SQL`` adhoc filters (and a + legacy top-level ``where``) join into ``extras.where`` / ``extras.having`` by + clause, so a chart restricted by a custom SQL predicate exports the same rows + it displays instead of the full, unrestricted result. + """ + where: list[str] = [] + having: list[str] = [] + if form_data.get("where"): + where.append(form_data["where"]) + for flt in form_data.get("adhoc_filters") or []: + if flt.get("expressionType") == "SQL" and flt.get("sqlExpression"): + clause = (flt.get("clause") or "WHERE").upper() + (having if clause == "HAVING" else where).append(flt["sqlExpression"]) + + extras: dict[str, str] = {} + if where: + extras["where"] = " AND ".join(f"({clause})" for clause in where) + if having: + extras["having"] = " AND ".join(f"({clause})" for clause in having) + return extras + + +def columns_from_form_data(form_data: dict[str, Any]) -> list[Any]: + """ + Derive the query's grouping/raw columns from form data. + + Handles raw-mode tables (``all_columns``/``columns``), an ``x_axis`` (string + or adhoc column), and ``groupby`` dimensions, de-duplicating while preserving + order. + """ + if form_data.get("query_mode") == "raw" and ( + form_data.get("all_columns") or form_data.get("columns") + ): + return list(form_data.get("all_columns") or form_data.get("columns") or []) + + groupby_columns: list[Any] = form_data.get("groupby") or [] + raw_columns: list[Any] = form_data.get("columns") or [] + # Prefer explicit raw columns only when they are actually present; a stale + # empty ``columns: []`` key must not shadow the group-by dimensions (which + # would silently drop the grouping and change the aggregation). + columns = raw_columns.copy() if raw_columns else groupby_columns.copy() + + x_axis = form_data.get("x_axis") + if isinstance(x_axis, str) and x_axis and x_axis not in columns: + columns.insert(0, x_axis) + elif isinstance(x_axis, dict): + col_name = x_axis.get("column_name") + if col_name and col_name not in columns: + columns.insert(0, col_name) Review Comment: **Suggestion:** An adhoc x-axis object representing a calculated column is reduced to only `column_name`, discarding its `sqlExpression` and other expression metadata. The rebuilt query then references the physical column name instead of the calculated expression, so legacy charts using SQL or calculated x-axis columns can fail or return different data. Preserve the full adhoc column definition, or explicitly reject this form when the builder cannot support it. [type error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Calculated-axis chart exports can omit their grouping. - ⚠️ Exported results can differ from chart visualization. - ⚠️ SQL-expression axes may cause query failures. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ca059bf18643404f80b0cf68c78043ab&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=ca059bf18643404f80b0cf68c78043ab&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/common/form_data_query_context.py **Line:** 121:127 **Comment:** *Type Error: An adhoc x-axis object representing a calculated column is reduced to only `column_name`, discarding its `sqlExpression` and other expression metadata. The rebuilt query then references the physical column name instead of the calculated expression, so legacy charts using SQL or calculated x-axis columns can fail or return different data. Preserve the full adhoc column definition, or explicitly reject this form when the builder cannot support it. 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%2F42284&comment_hash=5ddaa6ae3a9c4f68bbd4b94005d760ffba4bb908291381a4d765b2c169632a34&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=5ddaa6ae3a9c4f68bbd4b94005d760ffba4bb908291381a4d765b2c169632a34&reaction=dislike'>👎</a> ########## superset/common/form_data_query_context.py: ########## @@ -0,0 +1,273 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" +Synthesize a query context from a chart's saved form data (``params``). + +A chart's ``query_context`` is normally generated client-side by each viz +plugin's ``buildQuery`` and only persisted when the chart is (re-)saved in +Explore. Charts that predate that behavior keep their ``params`` (form data) but +carry no ``query_context``, so server-side consumers that need to run the query +(e.g. the dashboard Excel export) have nothing to execute. + +This module rebuilds a best-effort query context from the form data — columns, +metrics, filters (including free-form SQL and the time range), ordering and time +grain — mirroring the shared parts of the viz plugins' ``buildQuery``. It does +**not** reproduce plugin post-processing (pivot, contribution/percent +transforms, rolling/forecast) or multi-query fan-out, so callers must restrict it +to viz types whose data maps faithfully to a single plain query. +""" + +from __future__ import annotations + +from typing import Any + +from superset.utils import json + + +def adhoc_filters_to_query_filters( + adhoc_filters: list[dict[str, Any]], + where_only: bool = False, +) -> list[dict[str, Any]]: + """ + Convert ``SIMPLE`` adhoc filters into QueryObject filter clauses. + + Adhoc filters use ``{subject, operator, comparator}`` while a query object + expects ``{col, op, val}``; free-form ``SQL`` filters have no ``{col, op, + val}`` equivalent and are handled separately (see + :func:`freeform_where_having`). + + By default all ``SIMPLE`` filters are converted (the behavior the MCP + compile/preview path relies on). Pass ``where_only=True`` to convert only + ``WHERE``-clause filters, matching the frontend's ``processFilters`` — the + dashboard export uses this so it applies the same rows the chart shows and + does not additionally filter on ``SIMPLE`` ``HAVING`` clauses. + """ + result: list[dict[str, Any]] = [] + for flt in adhoc_filters or []: + if flt.get("expressionType") != "SIMPLE": + continue + if where_only and (flt.get("clause") or "WHERE").upper() != "WHERE": + continue + result.append( + { + "col": flt.get("subject"), + "op": flt.get("operator"), + "val": flt.get("comparator"), + } + ) + return result + + +def freeform_where_having(form_data: dict[str, Any]) -> dict[str, str]: + """ + Collect free-form SQL predicates into a query ``extras`` mapping. + + Mirrors ``processFilters`` on the frontend: ``SQL`` adhoc filters (and a + legacy top-level ``where``) join into ``extras.where`` / ``extras.having`` by + clause, so a chart restricted by a custom SQL predicate exports the same rows + it displays instead of the full, unrestricted result. + """ + where: list[str] = [] + having: list[str] = [] + if form_data.get("where"): + where.append(form_data["where"]) + for flt in form_data.get("adhoc_filters") or []: + if flt.get("expressionType") == "SQL" and flt.get("sqlExpression"): + clause = (flt.get("clause") or "WHERE").upper() + (having if clause == "HAVING" else where).append(flt["sqlExpression"]) + + extras: dict[str, str] = {} + if where: + extras["where"] = " AND ".join(f"({clause})" for clause in where) + if having: + extras["having"] = " AND ".join(f"({clause})" for clause in having) + return extras + + +def columns_from_form_data(form_data: dict[str, Any]) -> list[Any]: + """ + Derive the query's grouping/raw columns from form data. + + Handles raw-mode tables (``all_columns``/``columns``), an ``x_axis`` (string + or adhoc column), and ``groupby`` dimensions, de-duplicating while preserving + order. + """ + if form_data.get("query_mode") == "raw" and ( + form_data.get("all_columns") or form_data.get("columns") + ): + return list(form_data.get("all_columns") or form_data.get("columns") or []) + + groupby_columns: list[Any] = form_data.get("groupby") or [] + raw_columns: list[Any] = form_data.get("columns") or [] + # Prefer explicit raw columns only when they are actually present; a stale + # empty ``columns: []`` key must not shadow the group-by dimensions (which + # would silently drop the grouping and change the aggregation). + columns = raw_columns.copy() if raw_columns else groupby_columns.copy() + + x_axis = form_data.get("x_axis") + if isinstance(x_axis, str) and x_axis and x_axis not in columns: + columns.insert(0, x_axis) + elif isinstance(x_axis, dict): + col_name = x_axis.get("column_name") + if col_name and col_name not in columns: + columns.insert(0, col_name) + return columns + + +def is_raw_query_mode(form_data: dict[str, Any]) -> bool: + """ + Whether the chart runs in raw (non-aggregated) mode, mirroring the frontend's + ``getQueryMode``: an explicit ``query_mode`` wins, otherwise the presence of + ``all_columns`` implies raw mode. + """ + if mode := form_data.get("query_mode"): + return mode == "raw" + return bool(form_data.get("all_columns")) + + +def orderby_from_form_data( + form_data: dict[str, Any], metrics: list[Any], viz_type: str | None = None +) -> list[list[Any]]: + """ + Derive ordering so a ``row_limit`` returns the chart's top-N, not an + arbitrary N. + + Raw-mode tables order by ``order_by_cols`` (stored as JSON ``[col, asc]`` + pairs). Aggregate charts order by the configured sort metric + (``timeseries_limit_metric``, or the first metric when ``sort_by_metric`` is + set), otherwise fall back to the first metric descending — matching the + table/pie ``buildQuery`` defaults. + """ + if order_by_cols := form_data.get("order_by_cols") or []: + parsed: list[list[Any]] = [] + for col in order_by_cols: + if isinstance(col, str): + try: + col = json.loads(col) + except (TypeError, ValueError): + continue + parsed.append(col) + return parsed + + if not metrics: + return [] + + sort_metric = form_data.get("timeseries_limit_metric") or ( + metrics[0] if form_data.get("sort_by_metric") else None + ) + if sort_metric is not None: + # The Table plugin defaults ``order_desc`` to False (ascending); Pie and + # others sort by metric descending. Match that so a row limit keeps the + # chart's top/bottom-N rather than flipping it. + default_desc = viz_type != "table" + order_desc = form_data.get("order_desc", default_desc) + return [[sort_metric, not order_desc]] + # No explicit sort metric: default to the first metric, descending. + return [[metrics[0], False]] + + +def _columns_and_metrics( + form_data: dict[str, Any], viz_type: str | None +) -> tuple[list[Any], list[Any], bool]: + """ + Resolve the query's ``(columns, metrics, promoted_time_column)`` from form + data, honoring raw vs. aggregate mode and the Big Number trendline promotion. + """ + if is_raw_query_mode(form_data): + # Raw mode returns individual rows: use only the selected columns and + # ignore ``metrics``/``groupby``, which stay in form data as stale values + # (the controls aren't reset when hidden) but are ignored by the chart. + columns = list(form_data.get("all_columns") or form_data.get("columns") or []) + return columns, [], False + + metrics = list(form_data.get("metrics") or []) + # Single-metric charts (e.g. Big Number) store ``metric`` rather than + # ``metrics``. + if not metrics and form_data.get("metric"): + metrics = [form_data["metric"]] + columns = columns_from_form_data(form_data) + # Only a Big Number *with a trendline* (viz_type ``big_number``) groups by its + # time column; ``big_number_total`` is a single aggregate and must not be + # grouped, or it would return one row per timestamp instead of a total. + if not columns and viz_type == "big_number" and form_data.get("granularity_sqla"): + return [form_data["granularity_sqla"]], metrics, True + return columns, metrics, False + + +def build_query_context_from_form_data( + form_data: dict[str, Any], + datasource: dict[str, Any], + viz_type: str | None = None, +) -> dict[str, Any]: + """ + Build a query-context payload (the JSON shape ``ChartDataQueryContextSchema`` + loads) from a chart's form data and datasource reference. + + :param form_data: The chart's saved ``params`` parsed to a dict. + :param datasource: ``{"id": <int>, "type": "table"}`` datasource reference. + :param viz_type: The chart's viz type, used for viz-specific handling. + :returns: A single-query query-context dict. + """ + columns, metrics, promoted_time_column = _columns_and_metrics(form_data, viz_type) + + # SIMPLE adhoc filters (+ legacy top-level ``filters``) become query filters; + # free-form SQL predicates go into ``extras``. Only ``WHERE``-clause SIMPLE + # filters are applied (matching the chart), so the export never filters on a + # ``HAVING`` clause the chart itself ignores. + filters = adhoc_filters_to_query_filters( + form_data.get("adhoc_filters", []), where_only=True + ) + for flt in form_data.get("filters") or []: + if isinstance(flt, dict) and flt.get("col") is not None: + filters.append(flt) Review Comment: **Suggestion:** Legacy filter entries are accepted whenever they contain a `col`, even if they lack the required `op` or `val` fields. Such malformed entries later reach query-context processing, which indexes the missing filter fields and can raise an exception for the whole chart. Require a complete query-filter shape before appending legacy filters. [error handling] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Corrupt legacy filters can fail chart export. - ⚠️ Affected charts may be omitted from workbooks. - ⚠️ Malformed saved form data reaches query processing. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=11e6a83ab3a94a1abcbcda7d8dc4567c&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=11e6a83ab3a94a1abcbcda7d8dc4567c&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/common/form_data_query_context.py **Line:** 234:236 **Comment:** *Error Handling: Legacy filter entries are accepted whenever they contain a `col`, even if they lack the required `op` or `val` fields. Such malformed entries later reach query-context processing, which indexes the missing filter fields and can raise an exception for the whole chart. Require a complete query-filter shape before appending legacy filters. 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%2F42284&comment_hash=e883e605f7d2415345a2b8ddbd45d508cb63448e2b3fdfb70cc2d02614b4a181&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=e883e605f7d2415345a2b8ddbd45d508cb63448e2b3fdfb70cc2d02614b4a181&reaction=dislike'>👎</a> ########## superset/tasks/export_dashboard_excel.py: ########## @@ -96,6 +103,92 @@ def _chart_label(chart: Any) -> str: return f"{chart.id} - {chart.slice_name or ''}".strip() +def _saved_query_context(raw: Any) -> dict[str, Any] | None: + """ + The chart's saved query context parsed to a dict, or ``None`` when it is + missing or unusable. + + Returns ``None`` for a blank value, a string that does not parse as JSON, a + value that parses to something other than an object (e.g. ``"null"``), and an + object with no queries (e.g. ``"{}"`` or ``{"queries": []}``) — all treated + the same as a missing context. + """ + if not raw: + return None + try: + parsed = json.loads(raw) + except (TypeError, ValueError): + return None + if not isinstance(parsed, dict) or not parsed.get("queries"): + return None + return parsed + + +def _rebuild_viz_types() -> set[str]: + """Viz types eligible for form-data query-context rebuild (config or default). + + Only ``None`` falls back to the default; an explicitly configured empty set is + honored so operators can disable the rebuild entirely. + """ + configured = current_app.config.get("EXCEL_EXPORT_REBUILD_VIZ_TYPES") + return REBUILD_VIZ_TYPES if configured is None else configured + + +# Form-data keys whose behavior needs plugin post-processing or extra queries +# (contribution/time comparison, rolling window, resampling, raw big-number +# aggregation) that the single-query rebuild cannot reproduce. A chart using any +# of these is skipped rather than exported with values that differ from the chart. +_UNSUPPORTED_PROCESSING_KEYS = ("time_compare", "rolling_type", "resample_rule") + + +def _needs_unsupported_processing(form_data: dict[str, Any]) -> bool: + """Whether the form data relies on processing the rebuild can't reproduce.""" + # ``percent_metrics`` are "% of total" columns produced by contribution + # post-processing the rebuild can't apply; skip so the export doesn't silently + # omit columns the user sees. + if form_data.get("percent_metrics"): + return True + for key in _UNSUPPORTED_PROCESSING_KEYS: + value = form_data.get(key) + # ``rolling_type`` is often the literal string ``"None"`` when unset. + if value and value != "None": + return True + return form_data.get("aggregation") == "raw" + + +def _resolve_query_context(chart: Any) -> dict[str, Any] | None: + """ + The query-context payload to run for a chart's data export, or ``None`` when + none can be obtained. + + Prefers the chart's saved ``query_context``. When that is missing or empty, + synthesizes one from the chart's saved form data (``params``) — but only for + viz types whose data maps faithfully to a single plain query + (``EXCEL_EXPORT_REBUILD_VIZ_TYPES``) and that don't rely on post-processing or + extra queries the rebuild can't reproduce; other charts return ``None`` so the + caller lists them for re-saving rather than exporting inaccurate data. + """ + if saved := _saved_query_context(chart.query_context): + return saved + + if chart.viz_type not in _rebuild_viz_types() or chart.datasource_id is None: + return None + try: + form_data = json.loads(chart.params) if chart.params else {} + except (TypeError, ValueError): + return None + if not isinstance(form_data, dict) or not form_data: + return None + if _needs_unsupported_processing(form_data): + return None Review Comment: **Suggestion:** The default rebuild allowlist permits table charts, but the synthesized context does not reproduce table-specific extra queries such as `show_totals` or percent-metric calculations. An older allowlisted table using these options will export only the main query and silently omit the totals or derived data shown by the chart. Either detect these settings in `_needs_unsupported_processing` or include the required queries and post-processing before allowing the rebuild. [incomplete implementation] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Table Excel exports can omit configured totals rows. - ⚠️ Exported table data differs from displayed chart output. - ⚠️ Legacy charts are marked successful despite incomplete results. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=916fd9c6f5fd4f4fa211bc5c00ebed24&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=916fd9c6f5fd4f4fa211bc5c00ebed24&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/tasks/export_dashboard_excel.py **Line:** 174:183 **Comment:** *Incomplete Implementation: The default rebuild allowlist permits table charts, but the synthesized context does not reproduce table-specific extra queries such as `show_totals` or percent-metric calculations. An older allowlisted table using these options will export only the main query and silently omit the totals or derived data shown by the chart. Either detect these settings in `_needs_unsupported_processing` or include the required queries and post-processing before allowing the rebuild. 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%2F42284&comment_hash=b96030337b2b2d9fe9efa5569875ac857cea8e157445889f94f041ebf780d917&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42284&comment_hash=b96030337b2b2d9fe9efa5569875ac857cea8e157445889f94f041ebf780d917&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]
