sadpandajoe commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4221520152
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -1398,6 +2412,101 @@ def fallback_vega_lite_preview(
return None
+def generate_xy_pivot_vega_lite_preview(
+ data: list[dict[str, Any]], form_data: dict[str, Any], *, mark: str
+) -> VegaLitePreview | None:
+ """Render flattened timeseries pivot columns without dropping grouped
series.
+
+ Folding escaped field paths resolves literal output keys without splitting
+ category values that contain escaped commas. The legend retains each
+ complete metric/category label.
+ Long-form results continue through the generic renderer.
+ """
+ from superset.mcp_service.chart.chart_helpers import _as_list
+ from superset.utils.pandas_postprocessing.utils import (
+ escape_separator,
+ FLAT_COLUMN_SEPARATOR,
+ )
+
+ if not data:
+ return None
+ dimensions = [
+ label
+ for column in _as_list(form_data.get("groupby"))
+ if (label := _form_column_label(column))
+ ]
+ if not dimensions or any(label in data[0] for label in dimensions):
+ return None
+ x_axis = _form_column_label(form_data.get("x_axis")) or "__timestamp"
+ if x_axis not in data[0]:
+ return None
+ metric_labels = [
+ escape_separator(label)
Review Comment:
A grouped XY chart with a comma in a metric label (say `metrics: ["Revenue,
net"]`, `groupby: ["region"]`) won't get its category series. The result keys
were already unescaped to `Revenue, net, EU` by the query context processor,
but this prefix is built from the escaped `Revenue\, net`, so none of the
fields match, the adapter returns `None`, and the generic preview plots only
the first numeric series with no per-region coloring while still reporting
success. Should the prefix match use the unescaped label (and escape only when
building the Vega field path)?
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -1408,6 +1421,129 @@ Disabling a plugin only stops new charts of that type
from being created. Existi
- **[Security](/developer-docs/extensions/security)** -- Security best
practices for extensions
- **[Deployment](/developer-docs/extensions/deployment)** -- Package and
deploy Superset extensions
+## Bullet chart compatibility
+
+The MCP Bullet plugin uses `chart_type: "bullet"` and the native ECharts
+`viz_type: "bullet"`. Its optional `dimensions` hierarchy maps to `groupby`.
+Omit `dimensions` (or use `null`) to create a single-metric Bullet without a
+breakdown. On updates, omission or `null` preserves the saved hierarchy; use
+`dimensions: []` to clear it explicitly. An `order_by` update can reference the
+saved dimensions without resending them; unknown targets are rejected after
+resolving the saved hierarchy. When replacing the dataset, saved-chart and
+cached-preview updates retain an omitted hierarchy only if its columns resolve
+in the replacement dataset; incompatible inherited roles and temporal-filter
+provenance are discarded.
+
+Inherited native SQL dimensions remain query expressions when updating metrics
+or `order_by`; reference their output labels to sort by them. Caller-supplied
+`dimensions` must still be physical columns, not SQL expressions.
+
+Dimension and metric output names are case-sensitive: quoted physical columns
+such as `Region` and `region` remain distinct. Reference lookup prefers exact
+names and uses case-insensitive matching only when there is a single candidate;
+ambiguous references require the exact spelling. Bullet metric strings use
+JavaScript numeric spellings: underscore separators and non-ASCII digits are
+rejected rather than interpreted as numbers. Bounded array-valued dimensions
+retain their raw values in data reads and exports; preview category labels use
+JavaScript string conversion (for example, `[1, 2]` displays as `1,2`). Native
+SQL metrics without a label use their SQL expression as the output label.
+
+Presentation-only updates preserve saved predicates, including legacy top-level
+`where`, `having`, and `filters`, and normalize the native `order_by_cols`
alias.
+When both `orderby` and `order_by_cols` are saved, their sort entries are
+concatenated in form-data key order, including when `orderby` is empty.
+Explicit `filters: []` and `order_by: []` clear those inherited controls.
+
+Range, marker, and marker-line label lists may be shorter than their value
lists.
+Labels that become empty after sanitization retain their slots so later labels
+remain aligned with their corresponding values.
+As in Explore, missing or empty range labels are not displayed, and missing or
+empty marker labels use the formatted numeric value. Extra labels have no value
+to annotate and are ignored. These rules also apply to saved-chart previews and
+updates. Omitted label controls preserve the saved state only when their
+corresponding value controls (`ranges`, `markers`, or `marker_lines`) are also
+omitted. Replacing a value control without its labels clears the saved labels;
+resend the labels to retain annotations with the replacement values.
+
+Saved native `ranges`, `markers`, and `marker_lines` controls ignore empty,
+non-numeric, and NaN tokens, matching Explore. If no numeric range remains, the
+preview uses the default band up to 110% of the largest measure. Unrelated
+updates preserve these saved controls. Newly authored typed lists must contain
+finite numbers; infinite native values and oversized controls remain errors.
+
+On same-dataset chart updates, `filters: []` clears both user filters and the
+generated dashboard-time binding. `temporal_column: null` clears only that
+generated binding, preserving user filters. Omitting those controls preserves
+the saved binding. Bullet Vega previews keep separate indexed rows even when
+dimension display labels are identical, and support both `SMART_NUMBER` and
+`SMART_NUMBER_SIGNED` number formats. Bullet preview numeric format
+precision is limited to 20 digits before formatting; raw data reads do not
+validate presentation formats.
+
+Native Bullet temporal filters retain the active filter's subject and range as
a
+pair; `No filter` placeholders do not supply the subject of another active
range.
+Typed configs support one such pair. Multiple active native temporal filters,
or
+conflicts with explicit `temporal_column`/`time_range`, are rejected rather
than
+silently dropping or moving a predicate.
+
+### Query result limits
+
+MCP chart tools accept up to **50,000 rows per query** and **100,000 total
rows**
+across all queries in one result. The aggregate counts each returned data row,
+including repeated rows in different queries; it does not use `rowcount` or
+`total_rows` metadata. The row-shaped `indexnames`
+array emitted by Chart Data uses the same 50,000-entry limit, rather than the
+4,096-entry limit for other metadata arrays. Index entries count toward the
+shared row-data work budget and retain the 1 MiB aggregate metadata byte limit.
+Multi-dimension pivot index tuples are serialized as JSON arrays. Nested
+containers within an index entry retain the standard container limits.
+
+The shared query-result validator also enforces **non-configurable hard
limits**:
+
+- **2,500,000 values/containers across all queries**. Each row object, cell
+ scalar (including null), and nested list, tuple, array, or object contributes
+ to this shared work budget; nested elements count individually and repeated
+ occurrences count again. Object keys count toward byte limits, not this value
+ count. Row-shaped `indexnames` and their entries also consume this budget.
+ For example, 50,000 rows with 50 scalar columns require 2,550,000 values
+ (50,000 row objects plus 2,500,000 cells) and exceed the budget even if their
+ encoded size is below 16 MiB.
+- **64 KiB (65,536 UTF-8 bytes) per row-cell string value**. A single
+ 65,537-byte cell fails even in a one-row result. Binary cells must also fit
+ the cell limit before and after conversion to UTF-8 or a `base64:` string.
+- **16 MiB (16,777,216 bytes) of aggregate JSON-encoded result data and
+ metadata** across all queries, including escaped strings, keys, and container
+ syntax. Metadata strings (including SQL text) are not subject to the row-cell
+ string cap; all metadata shares a separate 1 MiB aggregate allowance.
+
+These limits apply before rendering or exporting in `get_chart_data`,
+`get_chart_preview`, chart generation/update compile checks, `query_dataset`,
+and semantic-layer `get_table`. They include inline/table responses and MCP
+CSV, Excel, and Parquet export paths; exports do not bypass source validation.
Review Comment:
This says the limits cover MCP Parquet export paths (`UPDATING.md` line 41
repeats it), but no MCP tool exports Parquet: `GetChartDataRequest.format` only
accepts `json`, `csv`, and `excel`, and `query_dataset`/`get_table` have no
export option. An operator or client following this would try
`format="parquet"` and get a request-validation error. Can both mentions be
limited to CSV/Excel?
##########
superset/mcp_service/dataset/tool/query_dataset.py:
##########
@@ -390,23 +430,25 @@ async def query_dataset( # noqa: C901
% (len(data), len(raw_columns), query_duration_ms)
)
- return QueryDatasetResponse(
- from_dttm=query_result.get("from_dttm"),
- to_dttm=query_result.get("to_dttm"),
- dataset_id=dataset.id,
- dataset_name=dataset_name,
- columns=columns_meta,
- data=data,
- row_count=len(data),
- total_rows=query_result.get("rowcount"),
- summary=summary,
- performance=PerformanceMetadata(
- query_duration_ms=query_duration_ms,
- cache_status=cache_label,
- ),
- cache_status=cache_status,
- applied_filters=effective_filters,
- warnings=warnings,
+ return _bounded_response(
Review Comment:
Nothing exercises this `_bounded_response` guard through the tool. 255 rows
with one 64 KiB `category` cell each pass source-result validation, but
`columns[].sample_values` repeats those cells and pushes the full response past
16 MiB. The nearby test in `test_query_dataset.py` (around line 407) only
covers an oversized cell rejected before profiling, and the shared preflight
tests don't call this tool, so removing the wrapper would leave every test
green while oversized success responses go out. Could a registered-tool test
mock only query execution and assert a `MalformedQueryResult` error for that
payload?
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -809,10 +1738,564 @@ def build_mixed_timeseries_secondary(
return qd
-# Deck.gl viz types that conditionally set is_timeseries from time_grain_sqla
-_DECK_TIMESERIES_VIZ_TYPES: frozenset[str] = frozenset(
- {"deck_arc", "deck_path", "deck_polygon", "deck_scatter",
"deck_screengrid"}
-)
+def build_histogram_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Histogram buildQuery, including its histogram post-processing."""
+ column = form_data.get("column")
+ histogram_groupby = _as_list(form_data.get("groupby"))
+ query = build_single_query_dict(
+ form_data,
+ [*histogram_groupby, column] if column is not None else
histogram_groupby,
+ [],
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ having_filter = bool(form_data.get("having")) or any(
+ isinstance(filter_, dict) and filter_.get("clause") == "HAVING"
+ for filter_ in form_data.get("adhoc_filters") or []
+ )
+ if having_filter:
+ query["metrics"] = [
+ {
+ "expressionType": "SQL",
+ "sqlExpression": "COUNT(*)",
+ "label": "COUNT(*)",
+ }
+ ]
+ bins = form_data.get("bins", 5)
+ try:
+ parsed_bins = float(bins)
+ parsed_bins = int(parsed_bins) if parsed_bins.is_integer() else
parsed_bins
+ except (TypeError, ValueError):
+ parsed_bins = 5
+ query["post_processing"] = [
+ {
+ "operation": "histogram",
+ "options": {
+ "column": _column_label(column),
+ "groupby": [
+ label
+ for item in histogram_groupby
+ if (label := _column_label(item))
+ ],
+ "bins": parsed_bins,
+ "cumulative": bool(form_data.get("cumulative")),
+ "normalize": bool(form_data.get("normalize")),
+ },
+ }
+ ]
+ return [query]
+
+
+def build_box_plot_query_dicts( # noqa: C901
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Box Plot buildQuery, including its boxplot post-processing."""
+ distribute = _as_list(form_data.get("columns"))
+ if not distribute and form_data.get("granularity_sqla"):
+ distribute = [form_data["granularity_sqla"]]
+ box_groupby = _as_list(form_data.get("groupby"))
+ query = build_single_query_dict(
+ form_data,
+ [
+ *(_temporal_column(column, form_data) for column in distribute),
+ *box_groupby,
+ ],
+ list(form_data.get("metrics") or []),
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ query["series_columns"] = box_groupby
+ if whisker := form_data.get("whiskerOptions"):
+ whisker_type = "tukey"
+ percentiles: list[int] | None = None
+ if whisker == "Min/max (no outliers)":
+ whisker_type = "min/max"
+ elif match := re.fullmatch(r"(\d{1,3})/(\d{1,3}) percentiles",
str(whisker)):
+ whisker_type = "percentile"
+ percentiles = [int(match.group(1)), int(match.group(2))]
+ elif whisker != "Tukey":
+ raise ValueError(f"Unsupported whisker type: {whisker}")
+ query["post_processing"] = [
+ {
+ "operation": "boxplot",
+ "options": {
+ "whisker_type": whisker_type,
+ "percentiles": percentiles,
+ "groupby": [
+ label
+ for column in box_groupby
+ if (label := _column_label(column))
+ ],
+ "metrics": [
+ label
+ for metric in query["metrics"]
+ if (label := _metric_label(metric))
+ ],
+ },
+ }
+ ]
+ return [query]
+
+
+def build_pivot_table_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Pivot Table buildQuery, including subtotal grouping sets."""
+ rows = _as_list(form_data.get("groupbyRows"))
+ pivot_columns = _as_list(form_data.get("groupbyColumns"))
+ if form_data.get("transposePivot"):
+ rows, pivot_columns = pivot_columns, rows
+ columns = _dedupe_query_fields([*rows, *pivot_columns], _column_label)
+ query = build_single_query_dict(
+ form_data,
+ [_temporal_column(column, form_data) for column in columns],
+ list(form_data.get("metrics") or []),
+ row_limit=row_limit,
+ order_desc=order_desc,
+ )
+ sort_metric = query.get("series_limit_metric")
+ if sort_metric is None and query["metrics"]:
+ sort_metric = query["metrics"][0]
+ if sort_metric is not None:
+ query["orderby"] = [[sort_metric, not query.get("order_desc", True)]]
+ if grouping_sets := _pivot_grouping_sets(form_data, rows, pivot_columns):
+ query["grouping_sets"] = grouping_sets
+ return [query]
+
+
+def build_pie_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ contribution: bool,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Pie/Sunburst buildQuery; Pie adds a contribution operator."""
+ metric = form_data.get("metric")
+ query = build_single_query_dict(
+ form_data,
+ _as_list(form_data.get("groupby")),
+ [metric] if metric is not None else [],
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=form_data.get("orderby"),
+ )
+ if form_data.get("sort_by_metric") and metric is not None:
+ query["orderby"] = [[metric, False]]
+ if contribution and (label := _metric_label(metric)):
+ query["post_processing"] = [
+ {
+ "operation": "contribution",
+ "options": {
+ "columns": [label],
+ "rename_columns": [f"{label}__contribution"],
+ },
+ }
+ ]
+ return [query]
+
+
+def _positive_int(value: Any) -> int:
+ """Coerce a stored limit (int, numeric string, or empty) to a positive int
or 0."""
+ try:
+ coerced = int(value)
+ except (TypeError, ValueError):
+ return 0
+ return coerced if coerced > 0 else 0
+
+
+def build_table_query_dicts( # noqa: C901
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Table buildQuery: percent metrics, comparisons, totals,
paging."""
+ raw_mode = form_data.get("query_mode") == "raw" or (
+ form_data.get("query_mode") not in {"raw", "aggregate"}
+ and bool(form_data.get("all_columns"))
+ )
+ # Native extractQueryFields excludes empty-string column references.
+ table_columns = [
+ column
+ for column in _as_list(
+ form_data.get("all_columns") if raw_mode else
form_data.get("groupby")
+ )
+ if column != ""
+ ]
+ table_metrics = [] if raw_mode else _as_list(form_data.get("metrics"))
+ percent_metrics = [] if raw_mode else
_as_list(form_data.get("percent_metrics"))
+ query_metrics = _dedupe_query_fields(
+ [*table_metrics, *percent_metrics], _metric_label
+ )
+ table_orderby = _parse_orderby(form_data.get("order_by_cols"))
+ if not raw_mode:
+ sort_metrics = _as_list(form_data.get("timeseries_limit_metric"))
+ if sort_metrics:
+ table_orderby = [[sort_metrics[0], not form_data.get("order_desc",
False)]]
+ elif table_metrics:
+ table_orderby = [[table_metrics[0], False]]
+ query = build_single_query_dict(
+ form_data,
+ table_columns,
+ query_metrics,
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=table_orderby,
+ )
+ if not raw_mode:
+ # Table selects one temporal axis and places it before the other roles.
+ for index, column in enumerate(table_columns):
+ temporal_column = _temporal_column(column, form_data)
+ if temporal_column is not column:
+ query["columns"] = [
+ temporal_column,
+ *table_columns[:index],
+ *table_columns[index + 1 :],
+ ]
+ break
+ # Native comparisons use ordinary metrics, before percentage-only metrics
+ # are added to the selected query and contribution operator.
+ has_time_comparison = _time_comparison(form_data, table_metrics)
+ offsets = _table_time_offsets(form_data, {**query, "metrics":
table_metrics})
+ query["time_offsets"] = offsets
+ post_processing: list[dict[str, Any]] = []
+ contribution: dict[str, Any] | None = None
+ if percent_metrics:
+ labels: list[str] = []
+ for metric in percent_metrics:
+ if label := _metric_label(metric):
+ candidates = [label]
+ if has_time_comparison:
+ candidates.extend(f"{label}__{offset}" for offset in
offsets)
+ for candidate in candidates:
+ if candidate not in labels:
+ labels.append(candidate)
+ contribution = {
+ "operation": "contribution",
+ "options": {
+ "columns": labels,
+ "rename_columns": [f"%{label}" for label in labels],
+ },
+ }
+ post_processing.append(contribution)
+ if has_time_comparison and offsets and form_data.get("comparison_type") !=
"values":
+ source: list[str] = []
+ shifted: list[str] = []
+ for metric in table_metrics:
+ if label := _metric_label(metric):
+ for offset in offsets:
+ source.append(label)
+ shifted.append(f"{label}__{offset}")
+ post_processing.append(
+ {
+ "operation": "compare",
+ "options": {
+ "source_columns": source,
+ "compare_columns": shifted,
+ "compare_type": form_data.get("comparison_type"),
+ "drop_original_columns": True,
+ },
+ }
+ )
+ query["post_processing"] = post_processing
+
+ # ``query["row_limit"]`` is the normalized caller limit (explicit request
+ # limit or the saved row_limit, which may be stored as a string); page
+ # sizing narrows it but never replaces it.
+ configured_limit = _positive_int(query.get("row_limit"))
+ if form_data.get("server_pagination"):
+ if page_size := _positive_int(form_data.get("server_page_length")):
+ query["row_limit"] = (
+ min(page_size, configured_limit) if configured_limit else
page_size
+ )
+ query["row_offset"] = 0
+
+ extra_queries: list[dict[str, Any]] = []
+ if form_data.get("percent_metric_calculation") == "all_records" and
percent_metrics:
+ extra_queries.append(
+ {
+ **query,
+ "columns": [],
+ "metrics": percent_metrics,
+ "post_processing": [],
+ "row_limit": 0,
+ "row_offset": 0,
+ "orderby": [],
+ "is_timeseries": False,
+ }
+ )
+ if query_metrics and form_data.get("show_totals") and not raw_mode:
+ totals = {
+ **query,
+ "columns": [],
+ "metrics": _table_totals_metrics(
+ query_metrics, form_data.get("totals_aggregate")
+ ),
+ "row_limit": 0,
+ "row_offset": 0,
+ "post_processing": [contribution] if contribution else [],
+ }
+ totals.pop("orderby", None)
+ totals.pop("order_desc", None)
+ extra_queries.append(totals)
+ if form_data.get("server_pagination"):
+ rowcount = {
+ **query,
+ "time_offsets": [],
+ "row_limit": configured_limit or 0,
+ "row_offset": 0,
+ "post_processing": [],
+ "is_rowcount": True,
+ }
+ return [query, rowcount, *extra_queries]
+ return [query, *extra_queries]
+
+
+def build_gantt_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Gantt buildQuery with its interval columns and series."""
+ (
+ gantt_columns,
+ gantt_metrics,
+ gantt_orderby,
+ gantt_groupby,
+ ) = resolve_gantt_query_fields(form_data)
+ query = build_single_query_dict(
+ form_data,
+ gantt_columns,
+ gantt_metrics,
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=gantt_orderby,
+ )
+ query["series_columns"] = gantt_groupby
+ return [query]
+
+
+def build_interactive_pivot_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Interactive Pivot Table buildQuery."""
+ interactive_columns = [
+ _temporal_column(column, form_data)
+ for column in _as_list(form_data.get("groupby"))
+ ]
+ query = build_single_query_dict(
+ form_data,
+ interactive_columns,
+ list(form_data.get("metrics") or []),
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=form_data.get("orderby"),
+ )
+ _normalize_orderby(query)
+ return [query]
+
+
+def build_big_number_query_dicts( # noqa: C901
+ form_data: dict[str, Any],
+ *,
+ trendline: bool,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Big Number (with or without trendline) buildQuery."""
+ metric = form_data.get("metric")
+ if metric is None:
+ plural_metrics = _as_list(form_data.get("metrics"))
+ metric = plural_metrics[0] if plural_metrics else None
+ columns = _resolve_big_number_query_columns(form_data) if trendline else []
+ query = build_single_query_dict(
+ form_data,
+ columns,
+ [metric] if metric is not None else [],
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=form_data.get("orderby"),
+ )
+ if trendline:
+ # Big Number has no series dimension. Its frontend pivot receives
+ # the common base QueryObject (whose columns are empty), not the
+ # final QueryObject after the explicit x-axis is added. Preserve
+ # that distinction instead of falling back to the final columns.
+ query["series_columns"] = []
+ if not form_data.get("x_axis"):
+ query["is_timeseries"] = True
+ query["post_processing"] = _timeseries_post_processing(form_data,
query)
+ if form_data.get("aggregation") == "raw":
+ return [
+ query,
+ {
+ **query,
+ "columns": [],
+ "is_timeseries": False,
+ "post_processing": [],
+ },
+ ]
+ return [query]
+
+
+def build_waterfall_query_dicts(
+ form_data: dict[str, Any],
+ *,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+) -> list[dict[str, Any]]:
+ """Render Waterfall buildQuery with raw-axis ordering."""
+ metrics, groupby = resolve_metrics_and_groupby(form_data)
+ # normalizeTimeColumn runs after Waterfall's buildQuery callback. It
+ # wraps only the final x-axis column; orderby deliberately retains the
+ # raw control value produced inside the callback.
+ raw_axis = form_data.get("x_axis") or form_data.get("granularity_sqla")
+ query_axis = (
+ _normalized_x_axis_query_field(form_data)
+ if form_data.get("x_axis")
+ else raw_axis
+ )
+ waterfall_columns = ([query_axis] if query_axis else []) + groupby
+ raw_ordering_columns = ([raw_axis] if raw_axis else []) + groupby
+ query = build_single_query_dict(
+ form_data,
+ waterfall_columns,
+ metrics,
+ row_limit=row_limit,
+ order_desc=order_desc,
+ orderby=None,
+ )
+ query["orderby"] = [[column, True] for column in raw_ordering_columns]
+ if form_data.get("x_axis"):
+ query.pop("is_timeseries", None)
+ return [query]
+
+
+def build_mixed_timeseries_query_dicts( # noqa: C901
Review Comment:
This new builder replaces `build_mixed_timeseries_secondary` (line 1695) and
`with_x_axis_column` (line 2398), which no longer have any caller now that the
Mixed Timeseries plugin uses it. The old builder lacks the shared-filter
reconciliation and post-processing of the active one, so a later fix for
secondary-layer filtering or ordering applied to the old one would change no
MCP output. Can these two helpers be removed?
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -760,7 +1639,56 @@ def build_single_query_dict(
order_desc if order_desc is not None else
form_data.get("order_desc", True)
)
qd["orderby"] = [(sort_metric, not descending)]
+ if orderby:
+ qd["orderby"] = orderby
+ for key in (
+ "annotation_layers",
+ "row_offset",
+ "series_columns",
+ "group_others_when_limit_reached",
+ "is_timeseries",
+ "time_offsets",
+ "time_compare_full_range",
+ ):
+ if key in form_data and form_data[key] is not None:
+ qd[key] = form_data[key]
+
+ # ``buildQueryObject`` keeps the modern series-limit controls, falls back
+ # to their legacy Timeseries names, and defaults the limit to zero. A
+ # malformed modern metric does not mask a valid legacy metric.
+ series_limit = form_data.get("series_limit")
+ if series_limit is None:
+ series_limit = form_data.get("limit")
+ if series_limit is not None:
+ qd["series_limit"] = series_limit
+ series_limit_metric = form_data.get("series_limit_metric")
+ if not _is_query_form_metric(series_limit_metric):
+ series_limit_metric = form_data.get("timeseries_limit_metric")
+ if series_limit_metric is not None:
+ qd["series_limit_metric"] = series_limit_metric
+ if apply_chart_fields is not None:
+ apply_chart_fields(qd, effective_row_limit)
apply_form_data_filters_to_query(qd, form_data)
+ # Mirror the common ``buildQueryObject``/``extractExtras`` translation used
+ # by native frontend plugins. ``granularity_sqla`` is a form-data control,
+ # while QueryObject calls the field ``granularity``; the SQL time grain is
+ # carried inside ``extras`` rather than as a top-level query field.
+ if include_common_temporal:
+ granularity = form_data.get("granularity") or
form_data.get("granularity_sqla")
+ if granularity:
+ qd["granularity"] = granularity
+ if time_grain := form_data.get("time_grain_sqla"):
+ qd["extras"] = {
+ **(qd.get("extras") or {}),
+ "time_grain_sqla": time_grain,
Review Comment:
For a semantic-view Table in aggregate mode grouped by a non-temporal column
(`temporal_columns_lookup: {"country": false}`) with a cached `time_grain_sqla:
"P1D"`, this copies the grain into `extras`. Explore's Table `buildQuery` drops
it through `omitDormantGrain`, but this reconstruction doesn't, so
`get_chart_data` fails in the semantic layer with "A time column must be
specified when a time grain is provided." for a chart that renders in Explore.
Should the Table path apply the same dormant-grain removal for semantic-view
datasources?
--
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]
