aminghadersohi commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4180329043
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1758,733 @@ def map_histogram_config(config:
"HistogramChartConfig") -> Dict[str, Any]:
return form_data
+def _bullet_token_list(values: Sequence[str | int | float]) -> str:
+ """Serialize typed Bullet controls to the frontend's comma-separated
form."""
+ tokens: list[str] = []
+ for value in values:
+ if isinstance(value, float):
+ token = repr(value)
+ # ``100`` parses back to the same binary float as ``100.0`` and
+ # preserves the frontend's established compact integer spelling.
+ if token.endswith(".0") and not (
+ value == 0.0 and math.copysign(1.0, value) < 0
+ ):
+ token = token[:-2]
+ tokens.append(token)
+ else:
+ tokens.append(str(value))
+ return ",".join(tokens)
+
+
+def map_bullet_config(config: BulletChartConfig) -> Dict[str, Any]: # noqa:
C901
+ """Map typed Bullet config to ``Bullet/buildQuery`` and transformProps.
+
+ The frontend buildQuery replaces the generic query fields with exactly one
+ metric and the groupby hierarchy. Presentation controls stay in native
+ snake_case form_data; the chart plugin camelizes them for transformProps.
+ """
+ if config.dimensions is None and config.order_by:
+ # An update resolves its saved hierarchy before mapping. Without one,
+ # creation must validate sort targets against an empty hierarchy.
+ BulletChartConfig.model_validate(
+ {**config.model_dump(exclude_unset=True), "dimensions": []}
+ )
+ metric = create_metric_object(config.metric)
+ form_data: Dict[str, Any] = {
+ "viz_type": "bullet",
+ "metric": metric,
+ }
+
+ # Optional semantic/query fields are emitted only when explicitly supplied.
+ # This lets update_chart and update_chart_preview preserve native saved
state,
+ # while an explicit empty value still clears it through the generic merge
path.
+ if "dimensions" in config.model_fields_set:
+ form_data["groupby"] = [dimension.name for dimension in
config.dimensions or []]
+ if "row_limit" in config.model_fields_set:
+ form_data["row_limit"] = config.row_limit
+ if "time_range" in config.model_fields_set:
+ form_data["time_range"] = config.time_range
+
+ if config.order_by:
+ dimensions = config.dimensions or []
+ orderby: list[list[Any]] = []
+ for order in config.order_by:
+ role, index = resolve_bullet_order_target(
+ order.column, dimensions, config.metric
+ )
+ if role == "metric":
+ order_target: Any = metric
+ else:
+ if index is None: # Defensive: resolver pairs dimensions with
indexes.
+ raise ValueError("Bullet dimension order target has no
index")
+ order_target = dimensions[index].name
+ orderby.append([order_target, order.ascending])
+ form_data["orderby"] = orderby
+ elif "order_by" in config.model_fields_set:
+ form_data["orderby"] = []
+
+ presentation_fields: dict[str, tuple[str, Any]] = {
+ "ranges": ("ranges", _bullet_token_list(config.ranges)),
+ "range_labels": (
+ "range_labels",
+ _bullet_token_list(config.range_labels),
+ ),
+ "markers": ("markers", _bullet_token_list(config.markers)),
+ "marker_labels": (
+ "marker_labels",
+ _bullet_token_list(config.marker_labels),
+ ),
+ "marker_lines": (
+ "marker_lines",
+ _bullet_token_list(config.marker_lines),
+ ),
+ "marker_line_labels": (
+ "marker_line_labels",
+ _bullet_token_list(config.marker_line_labels),
+ ),
+ "y_axis_format": ("y_axis_format", config.y_axis_format),
+ "show_labels": ("show_labels", config.show_labels),
+ "show_legend": ("show_legend", config.show_legend),
+ }
+ for field_name, (form_key, value) in presentation_fields.items():
+ if field_name in config.model_fields_set:
+ form_data[form_key] = value
+
+ _add_adhoc_filters(form_data, config.filters)
+ if config.filters == [] and "filters" in config.model_fields_set:
+ form_data["adhoc_filters"] = []
+ if config.time_range and config.temporal_column:
+ _ensure_temporal_adhoc_filter(form_data, config.temporal_column)
+ for filter_ in form_data.get("adhoc_filters", []):
+ if (
+ isinstance(filter_, dict)
+ and filter_.get("operator") ==
FilterOperator.TEMPORAL_RANGE.value
+ and filter_.get("subject") == config.temporal_column
+ and filter_.get("comparator") == NO_TIME_RANGE
+ ):
+ filter_["comparator"] = config.time_range
+ return form_data
+
+
+def merge_bullet_form_data(
+ existing_form_data: Mapping[str, Any], new_form_data: Dict[str, Any]
+) -> None:
+ """Preserve omitted native Bullet controls across update tool paths.
+
+ Query roles and every UI control have an explicit typed representation.
+ Mappers emit optional fields only when the caller supplied them, so copying
+ the bounded native keys below preserves omitted state while explicit empty,
+ false, null, and zero-like values remain authoritative.
+ """
+ if (
+ existing_form_data.get("viz_type") != "bullet"
+ or new_form_data.get("viz_type") != "bullet"
+ ):
+ return
+ preserved_keys = {
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Native predicates and
ordering aliases are normalized before merging; explicit clears remain
authoritative. Regression coverage is included.
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -809,10 +1739,551 @@ 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"))
+ )
+ table_columns = list(
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Table scalar columns and
metrics now use scalar-to-list normalization, including raw columns. Regression
coverage is included.
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -589,23 +591,518 @@ def resolve_big_number_columns(form_data: dict[str,
Any]) -> list[Any]:
return [granularity] if isinstance(granularity, str) and granularity else
[]
-def resolve_gantt_query_fields( # noqa: C901
- form_data: dict[str, Any],
-) -> tuple[list[Any], list[Any], list[list[Any]], list[Any]]:
- """Mirror the ECharts Gantt ``buildQuery`` field extraction contract.
+def extract_x_axis_col(form_data: dict[str, Any]) -> str | None:
+ """Return the x_axis column name from form_data, or None if not set."""
+ x_axis = form_data.get("x_axis")
+ if isinstance(x_axis, str) and x_axis:
+ return x_axis
+ if isinstance(x_axis, dict):
+ col_name = x_axis.get("column_name")
+ return col_name if isinstance(col_name, str) and col_name else None
+ return None
+
- Returns ``(columns, metrics, orderby, series_columns)``. Saved form data is
- user-editable, so malformed or oversized native ordering is rejected rather
- than silently dropped or passed into ``QueryContextFactory``.
+def _x_axis_query_field(form_data: dict[str, Any]) -> Any | None:
+ """Resolve a frontend x-axis value without losing SQL expressions."""
+ x_axis = form_data.get("x_axis")
+ if isinstance(x_axis, str) and x_axis:
+ return x_axis
+ if isinstance(x_axis, dict):
+ if (
+ isinstance(x_axis.get("sqlExpression"), str)
+ and x_axis.get("sqlExpression")
+ and isinstance(x_axis.get("label"), str)
+ and x_axis.get("label")
+ and x_axis.get("expressionType") in (None, "SQL")
+ ):
+ return x_axis
+ column_name = x_axis.get("column_name") or x_axis.get("columnName")
+ if isinstance(column_name, str) and column_name:
+ return column_name
+ return None
+
+
+def _normalized_x_axis_query_field(form_data: dict[str, Any]) -> Any | None:
+ """Mirror ``buildQueryContext.normalizeTimeColumn`` for a set x-axis."""
+ x_axis = _x_axis_query_field(form_data)
+ if x_axis is None:
+ return None
+ time_grain = form_data.get("time_grain_sqla")
+ if isinstance(x_axis, str):
+ normalized = {
+ "columnType": "BASE_AXIS",
+ "sqlExpression": x_axis,
+ "label": x_axis,
+ "expressionType": "SQL",
+ "isColumnReference": True,
+ }
+ if time_grain is not None:
+ normalized["timeGrain"] = time_grain
+ return normalized
+ normalized = {"columnType": "BASE_AXIS", **x_axis}
+ # The original adhoc column's grain overrides the common control, matching
+ # the frontend spread order.
+ if "timeGrain" not in normalized and time_grain is not None:
+ normalized["timeGrain"] = time_grain
+ return normalized
+
+
+def _resolve_big_number_query_columns(form_data: dict[str, Any]) -> list[Any]:
+ """Resolve only Big Number's explicit x-axis query column.
+
+ The frontend keeps ``granularity_sqla`` out of ``columns`` and asks the
+ backend for a timeseries instead, which yields ``__timestamp``. An explicit
+ ``x_axis`` is different: the plugin retains that column in the final query.
"""
- from superset.utils import json as utils_json
+ if (x_axis := _normalized_x_axis_query_field(form_data)) is not None:
+ return [x_axis]
+ return []
+
+
+def _as_list(value: Any) -> list[Any]:
+ """Match the frontend's ``ensureIsArray`` for query controls."""
+ if value is None:
+ return []
+ return value if isinstance(value, list) else [value]
+
+
+def _column_label(column: Any) -> str | None:
+ """Return the frontend ``getColumnLabel`` value for a query column."""
+ if isinstance(column, str):
+ return column
+ if not isinstance(column, dict):
+ return None
+ return (
+ column.get("label") or column.get("sqlExpression") or
column.get("column_name")
+ )
+
+
+def _metric_label(metric: Any) -> str | None:
+ """Return the frontend ``getMetricLabel`` value for a query metric."""
+ if isinstance(metric, str):
+ return metric
+ if not isinstance(metric, dict):
+ return None
+ if label := metric.get("label"):
+ return label
+ if metric.get("expressionType") == "SIMPLE":
+ column = metric.get("column") or {}
+ name = (
+ column.get("columnName") or column.get("column_name")
+ if isinstance(column, dict)
+ else None
+ )
+ if name and metric.get("aggregate"):
+ return f"{metric['aggregate']}({name})"
+ return metric.get("sqlExpression")
+
+
+def _is_query_form_metric(value: Any) -> bool:
+ """Mirror the frontend's ``isQueryFormMetric`` type guard."""
+ return isinstance(value, str) or (
+ isinstance(value, dict) and value.get("expressionType") in {"SIMPLE",
"SQL"}
+ )
+
+
+def _timeseries_base_metrics(form_data: dict[str, Any]) -> list[Any]:
+ """Return metrics extracted by the common frontend query-field aliases."""
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Timeseries query
reconstruction retains the singular metric alias and deduplicates metric
outputs. Regression coverage is included.
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -809,10 +1739,551 @@ 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"))
+ )
+ table_columns = list(
+ (form_data.get("all_columns") or [])
+ if raw_mode
+ else (form_data.get("groupby") or [])
+ )
+ table_metrics = [] if raw_mode else list(form_data.get("metrics") or [])
+ percent_metrics = [] if raw_mode else
_as_list(form_data.get("percent_metrics"))
+ table_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,
+ table_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
+ offsets = _table_time_offsets(form_data, query)
+ 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 offsets:
+ 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 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 table_metrics and form_data.get("show_totals") and not raw_mode:
+ totals = {
+ **query,
+ "columns": [],
+ "metrics": _table_totals_metrics(
+ table_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")
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Big Number query
reconstruction retains the plural-metric fallback. Regression coverage is
included.
##########
superset/mcp_service/chart/compile.py:
##########
@@ -282,6 +329,450 @@ def _validate_adhoc_filter_columns(
)
+def _native_validation_error(role: str, reference: str) ->
ChartGenerationError:
+ """Build a fail-closed error for an incompatible native chart reference."""
+ return ChartGenerationError(
+ error_type="invalid_native_chart_reference",
+ message=f"Native chart {role} {reference!r} is incompatible with the
dataset",
+ details=(
+ "The rebound form data must retain its exact query roles on the
target "
+ "dataset; no column or saved-metric reference may be guessed or
dropped."
+ ),
+ suggestions=[
+ "Choose a target dataset with a compatible schema",
+ "Provide a complete typed chart config using target-dataset
fields",
+ ],
+ error_code="CHART_VALIDATION_FAILED",
+ )
+
+
+def _native_column_name(value: Any) -> str | None:
+ """Extract a physical QueryFormColumn reference, or None for SQL
columns."""
+ if isinstance(value, str):
+ return value
+ if not isinstance(value, dict):
+ return None
+ if value.get("expressionType") == "SQL":
+ reference = value.get("sqlExpression")
+ if value.get("isColumnReference") is True and isinstance(reference,
str):
+ return reference or None
+ return None
+ name = value.get("column_name") or value.get("columnName")
+ return name if isinstance(name, str) and name else None
+
+
+def _native_column_label(value: Any) -> str | None:
+ """Return the frontend label for a native column without custom hooks."""
+ if isinstance(value, str):
+ return value
+ if not isinstance(value, dict):
+ return None
+ for key in ("label", "sqlExpression", "column_name", "columnName"):
+ candidate = value.get(key)
+ if isinstance(candidate, str) and candidate:
+ return candidate
+ return None
+
+
+def _native_metric_ref(value: Any) -> tuple[str, str] | None:
+ """Return ``(saved_metric|column, name)`` for a native query metric."""
+ if isinstance(value, str):
+ return "saved_metric", value
+ if not isinstance(value, dict):
+ return None
+ if value.get("expressionType") == "SQL":
+ return None
+ if value.get("expressionType") != "SIMPLE":
+ # Match QueryObject's guarded legacy saved-metric normalization.
+ if not ({"sqlExpression", "aggregate", "column"} & value.keys()):
+ label = value.get("label")
+ if isinstance(label, str) and label:
+ return "saved_metric", label
+ return None
+ column = value.get("column")
+ name = (
+ column.get("column_name") or column.get("columnName")
+ if isinstance(column, dict)
+ else None
+ )
+ return ("column", name) if isinstance(name, str) and name else None
+
+
+def _native_reference_error( # noqa: C901
+ form_data: Dict[str, Any],
+ dataset_context: DatasetContext,
+ dataset_id: int,
+ *,
+ strict_all_form_refs: bool,
+) -> ChartGenerationError | None:
+ """Validate the canonical native QueryObjects against a rebound dataset."""
+ from superset.mcp_service.chart.chart_helpers import (
+ build_query_dicts_from_form_data,
+ )
+
+ try:
+ queries = build_query_dicts_from_form_data(
+ deepcopy(form_data), dataset_id, "table"
+ )
+ except (KeyError, TypeError, ValueError) as ex:
+ return _native_validation_error("query contract",
safe_exception_message(ex))
+
+ saved_metrics = [item["name"] for item in
dataset_context.available_metrics]
+
+ def column_error(value: Any, role: str) -> ChartGenerationError | None:
+ name = _native_column_name(value)
+ if name is None:
+ if isinstance(value, dict) and value.get("expressionType") ==
"SQL":
+ return None
+ return _native_validation_error(role, repr(value)[:200])
+ try:
+ if resolve_dataset_column(name, dataset_context) is not None:
+ return None
+ except ValueError:
+ pass
+ return _native_validation_error(role, name)
+
+ def metric_error(value: Any, role: str) -> ChartGenerationError | None:
+ """Validate one raw or generated metric reference against the
target."""
+ ref = _native_metric_ref(value)
+ if ref is None:
+ if isinstance(value, dict) and value.get("expressionType") ==
"SQL":
+ return None
+ return _native_validation_error(role, repr(value)[:200])
+ kind, name = ref
+ if kind == "saved_metric":
+ # Native lookup selects an exact name unambiguously; only a
+ # case-folded reference has to be unique.
+ matches = (
+ [name]
+ if name in saved_metrics
+ else [
+ item for item in saved_metrics if item.casefold() ==
name.casefold()
+ ]
+ )
+ if len(set(matches)) != 1:
+ saved_role = f"{role.removesuffix(' metric')} saved metric"
+ return _native_validation_error(saved_role, name)
+ return None
+ return column_error(name, f"{role} column")
+
+ # Dataset-only rebind has no typed config to expose these native plugin
+ # roles. Validate the raw controls independently: some are consumed only
+ # while building ordering/post-processing and therefore may be absent from
+ # the final QueryObject (notably an explicit ordering can hide a ranking
+ # metric). Primary and secondary Mixed layers are deliberately separate.
+ viz_type = form_data.get("viz_type")
+ if strict_all_form_refs and (
+ viz_type == "mixed_timeseries"
+ or (
+ isinstance(viz_type, str)
+ and (
+ viz_type.startswith("echarts_timeseries") or viz_type ==
"echarts_area"
+ )
+ )
+ ):
+ if (raw_x_axis := form_data.get("x_axis")) is not None and (
+ error := column_error(raw_x_axis, "form-data x_axis column")
+ ):
+ return error
+ metric_fields = [
+ "metrics",
+ "size",
+ "timeseries_limit_metric",
+ "series_limit_metric",
+ ]
+ if viz_type == "mixed_timeseries":
+ metric_fields.extend(
+ [
+ "metrics_b",
+ "size_b",
+ "timeseries_limit_metric_b",
+ "series_limit_metric_b",
+ ]
+ )
+ for field_name in metric_fields:
+ raw_value = form_data.get(field_name)
+ values = raw_value if isinstance(raw_value, list) else [raw_value]
+ for value in values:
+ if value is not None and (
+ error := metric_error(value, f"form-data {field_name}
metric")
+ ):
+ return error
+
+ layer_suffixes = ("", "_b") if viz_type == "mixed_timeseries" else
("",)
+ for suffix in layer_suffixes:
+ sort_field = f"x_axis_sort{suffix}"
+ if sort_field not in form_data or form_data.get(sort_field) is
None:
+ continue
+ x_axis = form_data.get(f"x_axis{suffix}", form_data.get("x_axis"))
+ allowed_labels: set[str] = set()
+ if x_axis_label := _native_column_label(x_axis):
+ allowed_labels.add(x_axis_label)
+ raw_metrics = form_data.get(f"metrics{suffix}")
+ for metric in raw_metrics if isinstance(raw_metrics, list) else []:
+ if label := _metric_label_for_validation(metric):
+ allowed_labels.add(label)
+ raw_limit_metric =
form_data.get(f"timeseries_limit_metric{suffix}")
+ limit_metrics = (
+ raw_limit_metric
+ if isinstance(raw_limit_metric, list)
+ else [raw_limit_metric]
+ )
+ for metric in limit_metrics:
+ if label := _metric_label_for_validation(metric):
+ allowed_labels.add(label)
+ sort_value = form_data[sort_field]
+ if not isinstance(sort_value, str) or sort_value not in
allowed_labels:
+ return _native_validation_error(sort_field,
repr(sort_value)[:200])
+
+ if (
+ strict_all_form_refs
+ and isinstance(viz_type, str)
+ and viz_type.startswith("deck_")
+ ):
+ # Deck layers store most query roles outside common columns/metrics.
+ # Validate every renderer-consumed raw control as well as the generated
+ # QueryObject so a dataset-only rebind cannot hide or discard a stale
+ # tooltip, cross-filter, spatial, path, or metric reference.
+ for spatial_field in ("spatial", "start_spatial", "end_spatial"):
+ spatial = form_data.get(spatial_field)
+ if spatial is None:
+ continue
+ if not isinstance(spatial, dict):
+ return _native_validation_error(
+ f"form-data {spatial_field}", repr(spatial)[:200]
+ )
+ spatial_type = spatial.get("type")
+ if not isinstance(spatial_type, str):
+ return _native_validation_error(
+ f"form-data {spatial_field} type", repr(spatial_type)[:200]
+ )
+ role_fields = {
+ "latlong": ("lonCol", "latCol"),
+ "delimited": ("lonlatCol",),
+ "geohash": ("geohashCol",),
+ }.get(spatial_type)
+ if role_fields is None:
+ return _native_validation_error(
+ f"form-data {spatial_field} type", repr(spatial_type)[:200]
+ )
+ for role_field in role_fields:
+ spatial_value = spatial.get(role_field)
+ if spatial_value is None:
+ return _native_validation_error(
+ f"form-data {spatial_field}.{role_field} column",
"missing"
+ )
+ if error := column_error(
+ spatial_value, f"form-data {spatial_field}.{role_field}
column"
+ ):
+ return error
+
+ for field_name in (
+ "line_column",
+ "geojson",
+ "dimension",
+ "cross_filter_column",
+ ):
+ column_value = form_data.get(field_name)
+ if column_value is not None and (
+ error := column_error(column_value, f"form-data {field_name}
column")
+ ):
+ return error
+
+ tooltip_contents = form_data.get("tooltip_contents")
+ if tooltip_contents is not None and not isinstance(tooltip_contents,
list):
+ return _native_validation_error(
+ "form-data tooltip_contents", repr(tooltip_contents)[:200]
+ )
+ for index, item in enumerate(tooltip_contents or []):
+ tooltip_value: Any = None
+ if isinstance(item, str):
+ tooltip_value = item
+ elif isinstance(item, dict) and item.get("item_type") == "column":
+ tooltip_value = item.get("column_name")
+ if tooltip_value is not None and (
+ error := column_error(
+ tooltip_value, f"form-data tooltip_contents[{index}]
column"
+ )
+ ):
+ return error
+
+ metric_values: list[tuple[str, Any]] = []
+ if viz_type not in {"deck_geojson", "deck_polygon"}:
+ for field_name in ("metrics", "metric", "size"):
+ raw_deck_metrics = form_data.get(field_name)
+ deck_metrics = (
+ raw_deck_metrics
+ if isinstance(raw_deck_metrics, list)
+ else [raw_deck_metrics]
+ )
+ metric_values.extend(
+ (f"form-data {field_name} metric", deck_metric)
+ for deck_metric in deck_metrics
+ if deck_metric is not None
+ )
+ if viz_type == "deck_polygon" and form_data.get("metric") is not None:
+ metric_values.append(("form-data metric metric",
form_data.get("metric")))
+ fixed_metric_fields = (
+ ("point_radius_fixed",)
+ if viz_type in {"deck_scatter", "deck_polygon"}
+ else ()
+ ) + (("line_width",) if viz_type == "deck_path" else ())
+ for field_name in fixed_metric_fields:
+ fixed_value = form_data.get(field_name)
+ deck_metric: Any = (
+ fixed_value
+ if (
+ isinstance(fixed_value, str)
+ and fixed_value
+ and viz_type != "deck_polygon"
+ )
+ else None
+ )
+ if isinstance(fixed_value, dict) and fixed_value.get("type") ==
"metric":
+ deck_metric = fixed_value.get("value")
+ if deck_metric is not None:
+ metric_values.append((f"form-data {field_name} metric",
deck_metric))
+ if viz_type == "deck_path" and form_data.get("breakpoint_metric") is
not None:
+ metric_values.append(
+ (
+ "form-data breakpoint_metric metric",
+ form_data.get("breakpoint_metric"),
+ )
+ )
+ for role, deck_metric in metric_values:
+ if error := metric_error(deck_metric, role):
+ return error
+
+ for filter_ in form_data.get("adhoc_filters") or []:
+ if not isinstance(filter_, dict) or filter_.get("expressionType") !=
"SIMPLE":
+ continue
+ if not strict_all_form_refs and _is_inert_adhoc_filter(filter_):
+ continue
+ subject = filter_.get("subject")
+ clause = str(filter_.get("clause") or "WHERE").upper()
+ if clause == "HAVING" and isinstance(subject, str):
+ metric_matches = (
+ [subject]
+ if subject in saved_metrics
+ else [
+ name
+ for name in saved_metrics
+ if name.casefold() == subject.casefold()
+ ]
+ )
+ if len(metric_matches) == 1:
+ continue
+ if subject is not None and (
+ error := column_error(subject, "form-data filter column")
+ ):
+ return error
+ if filter_.get("operator") == "TEMPORAL_RANGE" and isinstance(subject,
str):
+ try:
+ temporal = resolve_dataset_column(subject, dataset_context)
+ except ValueError:
+ temporal = None
+ if temporal is not None and not temporal.get("is_temporal", False):
+ return _native_validation_error("temporal filter column",
subject)
+
+ # temporal_columns_lookup describes the entire datasource, not selected
+ # roles. The physical form/query column checks validate selected
references.
+
+ for query_index, query in enumerate(queries, 1):
+ metric_labels: set[str] = set()
+ for column in query.get("columns") or []:
+ if error := column_error(column, f"query {query_index} column"):
+ return error
+ for column in query.get("series_columns") or []:
+ if error := column_error(column, f"query {query_index} series
column"):
+ return error
+ for column in query.get("groupby") or []:
+ if error := column_error(column, f"query {query_index} groupby
column"):
+ return error
+ for level in query.get("grouping_sets") or []:
+ for column in level:
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Grouping sets resolve
selected logical column labels after their source columns are validated.
Regression coverage is included.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2713,6 +2820,465 @@ def validate_unique_column_labels(self) ->
"XYChartConfig":
return self
+class BulletChartConfig(BaseChartConfig):
+ """Config for bullet charts (viz_type ``bullet``)."""
+
+ # Semantic field names are exposed to MCP clients; validation aliases and
the
+ # native adapter accept saved Explore ``form_data`` without weakening the
+ # unknown-field checks that catch misspelled controls.
+ model_config = ConfigDict(extra="ignore", populate_by_name=True)
+
+ chart_type: Literal["bullet"] = "bullet"
+ metric: ColumnRef = Field(
+ ...,
+ description="Numeric measure shown by each bullet bar",
+ )
+ dimensions: List[ColumnRef] | None = Field(
+ None,
+ validation_alias=AliasChoices("dimensions", "groupby"),
+ description=(
+ "Category hierarchy; one bullet row per unique combination. Omit
to "
+ "keep the saved hierarchy on update; [] clears it."
+ ),
+ max_length=20,
+ )
+ filters: List[FilterConfig] | None = Field(None, max_length=100)
+ time_range: str | None = Field(
+ None,
+ min_length=1,
+ max_length=1000,
+ description=(
+ "Superset time range, e.g. 'Last 30 days' or '2025-01-01 :
2025-12-31'"
+ ),
+ )
+ row_limit: int = Field(
+ 10000,
+ ge=1,
+ le=50000,
+ description="Maximum bullet rows",
+ )
+ order_by: List[SortByConfig] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("order_by", "orderby", "order_by_cols"),
+ max_length=20,
+ description="Row order by a dimension name or the metric output
label/name",
+ )
+
+ # Presentation fields map one-for-one onto Bullet/transformProps.ts
controls.
+ ranges: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Qualitative range thresholds shaded behind the measure",
+ )
+ range_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("range_labels", "rangeLabels"),
+ max_length=100,
+ )
+ markers: List[float] = Field(
+ default_factory=list,
+ max_length=100,
+ description="Target values drawn as point markers",
+ )
+ marker_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_labels", "markerLabels"),
+ max_length=100,
+ )
+ marker_lines: List[float] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_lines", "markerLines"),
+ max_length=100,
+ description="Reference values drawn as vertical lines",
+ )
+ marker_line_labels: List[str] = Field(
+ default_factory=list,
+ validation_alias=AliasChoices("marker_line_labels",
"markerLineLabels"),
+ max_length=100,
+ )
+ y_axis_format: str = Field(
+ "SMART_NUMBER",
+ validation_alias=AliasChoices("y_axis_format", "yAxisFormat"),
+ max_length=100,
+ )
+ show_labels: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_labels", "showLabels"),
+ )
+ show_legend: bool = Field(
+ False,
+ validation_alias=AliasChoices("show_legend", "showLegend"),
+ )
+
+ @staticmethod
+ def _adapt_native_metric(value: Any) -> Any:
+ """Translate QueryFormMetric shapes into the shared ColumnRef
contract."""
+ if isinstance(value, str):
+ return {"name": value, "saved_metric": True}
+ if not isinstance(value, dict):
+ return value
+ if "expressionType" not in value:
+ # QueryObject's documented legacy saved-metric representation is a
+ # label-only object. Keep this adapter deliberately narrow: objects
+ # carrying ad-hoc fields must declare expressionType explicitly,
and
+ # semantic ColumnRef objects continue through normal validation.
+ if set(value) == {"label"}:
+ label = value["label"]
+ if not isinstance(label, str) or not label or len(label) > 255:
+ raise ValueError(
+ "legacy saved metric label must be a non-empty string
of "
+ "at most 255 characters"
+ )
+ return {"name": label, "saved_metric": True}
+ return value
+ expression_type = value.get("expressionType")
+ if expression_type == "SQL":
+ return {
+ "sql_expression": value.get("sqlExpression"),
+ "label": value.get("label"),
Review Comment:
Fixed in c2d1b3f6f86e588598905382052d36927595a40d. Native SQL metrics use
the SQL expression when the label is missing or null. Regression coverage is
included.
--
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]
