sadpandajoe commented on code in PR #43770:
URL: https://github.com/apache/superset/pull/43770#discussion_r4174947212
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2538,6 +2539,89 @@ def _metric_display_label(col: ColumnRef) -> str:
return col.label or col.name or ""
+def _require_unique_bullet_order_match(
+ matches: list[tuple[str, int | None]], target: str, role: str
+) -> tuple[str, int | None] | None:
+ """Return a unique sort-role match or reject ambiguity."""
+ if len(matches) == 1:
+ return matches[0]
+ if len(matches) > 1:
+ raise ValueError(f"ambiguous {role}: {target}")
+ return None
+
+
+def _bullet_metric_output_label(metric: ColumnRef) -> str:
+ """Return the field name emitted by Bullet's native metric shape."""
+ if metric.saved_metric:
+ # Saved metrics are serialized as a bare metric-name string; a
+ # ColumnRef display label does not change the query output alias.
+ return metric.name or ""
+ return _metric_display_label(metric)
Review Comment:
For `metric: {"name": "Revenue"}`, this validates the output as `Revenue`,
but the mapper emits `SUM(Revenue)`, so grouping by `Revenue` is rejected as a
collision and ordering by the actual metric output is rejected as unknown;
`STDDEV` similarly maps to `STDDEV_SAMP`. Could output-role validation use the
same defaulting and aggregate aliases as `create_metric_object`?
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -809,10 +1750,544 @@ 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:
+ query["columns"] = [
Review Comment:
With Table grouping on `created_at` and `updated_at` and a daily grain, this
wraps both temporal columns, whereas the frontend buckets only the first
eligible column. Rows with different `updated_at` hours on the same day are
therefore collapsed into one sum only in MCP; could this mirror the frontend's
single temporal-axis selection?
##########
superset/mcp_service/chart/plugins/bullet.py:
##########
@@ -0,0 +1,459 @@
+# 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.
+
+"""ECharts Bullet chart type plugin."""
+
+from __future__ import annotations
+
+from collections.abc import Callable, Mapping
+from typing import Any, ClassVar
+
+from superset.mcp_service.chart.chart_utils import (
+ _summarize_filters,
+ map_bullet_config,
+)
+from superset.mcp_service.chart.plugin import BaseChartPlugin
+from superset.mcp_service.chart.schemas import (
+ BulletChartConfig,
+ ChartError,
+ ColumnRef,
+ resolve_bullet_order_target,
+ VegaLitePreview,
+)
+from superset.mcp_service.chart.validation.dataset_validator import (
+ AmbiguousDatasetReferenceError,
+ DatasetValidator,
+ is_numeric_column,
+ resolve_dataset_column,
+)
+from superset.mcp_service.common.error_schemas import (
+ ChartGenerationError,
+ DatasetContext,
+)
+
+
+def _canonical_reference(
+ name: str,
+ candidates: list[str],
+ role: str,
+) -> str:
+ """Resolve exact/casefold matches without silently choosing ambiguity."""
+ if name in candidates:
+ return name
+ matches = [
+ candidate for candidate in candidates if candidate.casefold() ==
name.casefold()
+ ]
+ if len(matches) > 1:
+ # AmbiguousDatasetReferenceError subclasses ValueError, so existing
+ # ValueError handlers keep working, while the validation pipeline
+ # re-raises it instead of downgrading it to a warning and proceeding
+ # with the unresolved reference.
+ raise AmbiguousDatasetReferenceError(name, matches, f"Bullet {role}")
+ return matches[0] if matches else name
+
+
+def _render_model_error(rows: Any, form_data: Mapping[str, Any]) -> ChartError
| None:
+ """Return why ``rows`` cannot build the strict Bullet render model."""
+ from superset.mcp_service.chart.preview_utils import (
+ BulletOutputError,
+ resolve_bullet_render_model,
+ )
+ from superset.mcp_service.chart.query_result import safe_exception_message
+
+ try:
+ resolve_bullet_render_model(rows, dict(form_data))
+ except BulletOutputError as ex:
+ return ChartError(error=safe_exception_message(ex),
error_type=ex.error_type)
+ return None
+
+
+class BulletChartPlugin(BaseChartPlugin):
+ """Plugin matching ``plugin-chart-echarts/src/Bullet``."""
+
+ chart_type = "bullet"
+ display_name = "Bullet Chart"
+ native_viz_types: ClassVar[Mapping[str, str]] = {
+ "bullet": "Bullet Chart",
+ }
+ # Bullet/transformProps.ts converts temporal values with Number().
+ temporal_json_numbers = True
+ # The frontend renders an empty result (a zero measure when ungrouped).
+ allows_empty_result = True
+ allows_empty_data_result = True
+ # Updates merge filter provenance plus Bullet's bounded native controls;
+ # no other saved control is carried into the typed Bullet state.
+ owns_update_merge = True
+ binds_time_range_to_temporal_filter = True
+ table_preview_unsupported_reason: ClassVar[str | None] = (
+ "Table previews cannot represent Bullet ranges, markers, "
+ "labels, and legend semantics"
+ )
+ invalid_result_error_code = "MALFORMED_BULLET_OUTPUT"
+ invalid_result_message = (
+ "Bullet query output does not contain a usable sizing measure."
+ )
+
+ def pre_validate(self, config: dict[str, Any]) -> ChartGenerationError |
None:
+ if "metric" in config:
+ return None
+ return ChartGenerationError(
+ error_type="missing_bullet_fields",
+ message="Bullet chart missing required field: metric",
+ details=(
+ "A Bullet chart measures one numeric aggregate or saved/SQL
metric; "
+ "optional dimensions split it into one row per group."
+ ),
+ suggestions=[
+ "Add metric: {'name': 'revenue', 'aggregate': 'SUM'}",
+ "For a saved metric use {'name': 'revenue', 'saved_metric':
true}",
+ "Add dimensions: [{'name': 'region'}] for grouped bullet rows",
+ ],
+ error_code="MISSING_BULLET_FIELDS",
+ )
+
+ def extract_column_refs(self, config: Any) -> list[ColumnRef]:
+ if not isinstance(config, BulletChartConfig):
+ return []
+ refs = [config.metric, *(config.dimensions or [])]
+ refs.extend(ColumnRef(name=filter_.column) for filter_ in
config.filters or [])
+ # order_by is constrained to role outputs by the schema, so those names
+ # are already represented by metric/dimension refs and must not be
+ # reinterpreted as physical columns.
+ return refs
+
+ def to_form_data(
+ self, config: Any, dataset_id: int | str | None = None
+ ) -> dict[str, Any]:
+ if not isinstance(config, BulletChartConfig):
+ raise TypeError("BulletChartPlugin requires BulletChartConfig")
+ return map_bullet_config(config)
+
+ def post_map_validate( # noqa: C901
+ self,
+ config: Any,
+ form_data: dict[str, Any],
+ dataset_id: int | str | None = None,
+ ) -> ChartGenerationError | None:
+ """Require an unambiguous numeric metric output for Number(...)."""
+ if not isinstance(config, BulletChartConfig) or dataset_id is None:
+ return None
+ dataset_context = DatasetValidator._get_dataset_context(dataset_id)
+ if dataset_context is None:
+ return None
+
+ columns = [column["name"] for column in
dataset_context.available_columns]
+ metrics = [metric["name"] for metric in
dataset_context.available_metrics]
+ requested: list[tuple[str, list[str], str]] = []
+ if config.metric.name and not config.metric.sql_expression:
+ requested.append(
+ (
+ config.metric.name,
+ metrics if config.metric.saved_metric else columns,
+ "saved metric" if config.metric.saved_metric else "metric
column",
+ )
+ )
+ requested.extend(
+ (dimension.name or "", columns, "dimension")
+ for dimension in config.dimensions or []
+ if dimension.name
+ )
+ requested.extend(
+ (filter_.column, columns, "filter column")
+ for filter_ in config.filters or []
+ )
+ if config.temporal_column:
+ requested.append((config.temporal_column, columns, "temporal
column"))
+
+ for name, candidates, role in requested:
+ if (
+ name not in candidates
+ and sum(
+ candidate.casefold() == name.casefold() for candidate in
candidates
+ )
+ > 1
+ ):
+ return ChartGenerationError(
+ error_type="ambiguous_bullet_reference",
+ message=(
+ f"Bullet {role} {name!r} is ambiguous in dataset
metadata"
+ ),
+ details=(
+ "Multiple dataset fields differ only by case. The
query and "
+ "frontend require an exact canonical field name."
+ ),
+ suggestions=[
+ "Use get_dataset_info and copy the exact-case field
name"
+ ],
+ error_code="AMBIGUOUS_BULLET_REFERENCE",
+ )
+
+ metric = config.metric
+ if metric.saved_metric or metric.sql_expression:
+ # Saved/SQL metric result types are determined by their
expressions;
+ # Tier-2 compile validation remains authoritative.
+ return None
+ if (metric.aggregate or "SUM") in {"COUNT", "COUNT_DISTINCT"}:
+ return None
+ try:
+ column = resolve_dataset_column(metric.name or "", dataset_context)
+ except ValueError as ex:
+ return ChartGenerationError(
+ error_type="ambiguous_bullet_reference",
+ message=(
+ f"Bullet metric column {metric.name!r} is ambiguous in "
+ "dataset metadata"
+ ),
+ details=str(ex),
+ suggestions=["Use get_dataset_info and copy the exact-case
field name"],
+ error_code="AMBIGUOUS_BULLET_REFERENCE",
+ )
+ if column is None or is_numeric_column(column):
+ return None
+ return ChartGenerationError(
+ error_type="non_numeric_bullet_metric",
+ message=(
+ f"Bullet metric {metric.name!r} must produce a number; dataset
"
+ f"type is {column.get('type', 'UNKNOWN')}."
+ ),
+ details=(
+ "Bullet/transformProps.ts converts the metric result with
Number(). "
+ "A non-numeric MIN/MAX or default SUM would render an invalid
bar."
+ ),
+ suggestions=[
+ "Use COUNT or COUNT_DISTINCT for a text column",
+ "Choose a numeric dataset column",
+ "Use a saved or SQL metric that returns a numeric value",
+ ],
+ error_code="NON_NUMERIC_BULLET_METRIC",
+ )
+
+ def normalize_column_refs(
+ self, config: Any, dataset_context: DatasetContext
+ ) -> Any:
+ if not isinstance(config, BulletChartConfig):
+ return config
+ explicit_fields = set(config.model_fields_set)
+ config_dict = config.model_dump(exclude_unset=True)
+ columns = [column["name"] for column in
dataset_context.available_columns]
+ metrics = [metric["name"] for metric in
dataset_context.available_metrics]
+
+ metric = config_dict["metric"]
+ if not metric.get("sql_expression"):
+ metric["name"] = _canonical_reference(
+ metric["name"],
+ metrics if metric.get("saved_metric") else columns,
+ "saved metric" if metric.get("saved_metric") else "metric
column",
+ )
+ for dimension in config_dict.get("dimensions") or []:
+ dimension["name"] = _canonical_reference(
+ dimension["name"], columns, "dimension"
+ )
+ if temporal := config_dict.get("temporal_column"):
+ config_dict["temporal_column"] = _canonical_reference(
+ temporal, columns, "temporal column"
+ )
+ for filter_ in config_dict.get("filters") or []:
+ filter_["column"] = _canonical_reference(
+ filter_["column"], columns, "filter column"
+ )
+
+ # Sort targets may use ergonomic role names or labels. Canonicalize
+ # physical-name targets and leave explicit display labels untouched.
+ for order in config_dict.get("order_by") or []:
+ role, index = resolve_bullet_order_target(
+ order["column"], config.dimensions or [], config.metric
+ )
+ if role == "dimension" and index is not None:
+ order["column"] = config_dict["dimensions"][index]["name"]
+ elif not metric.get("sql_expression") and not metric.get("label"):
+ order["column"] = metric["name"]
+
+ normalized = BulletChartConfig.model_validate(config_dict)
+ normalized.model_fields_set.clear()
+ normalized.model_fields_set.update(explicit_fields)
+ return normalized
+
+ def generate_name(self, config: Any, dataset_name: str | None = None) ->
str:
+ metric = config.metric.label or config.metric.name or "Metric"
+ what = f"{metric} bullet"
+ if config.dimensions:
+ what += " by " + ", ".join(
+ dimension.label or dimension.name or "dimension"
+ for dimension in config.dimensions
+ )
+ return self._with_context(what, _summarize_filters(config.filters))
+
+ def resolve_viz_type(self, config: Any) -> str:
+ return "bullet"
+
+ def schema_error_hint(self) -> ChartGenerationError | None:
+ return ChartGenerationError(
+ error_type="bullet_validation_error",
+ message="Bullet chart configuration validation failed",
+ details=(
+ "Bullet requires one numeric metric and optional unique
physical "
+ "dimensions. Threshold/marker labels are optional; missing
marker "
+ "labels fall back to formatted values."
+ ),
+ suggestions=[
+ "Use metric with aggregate, saved_metric, or sql_expression +
label",
+ "Use dimensions (alias: groupby) for row hierarchy",
+ "Use ranges, markers, and marker_lines for comparison targets",
+ ],
+ error_code="BULLET_VALIDATION_ERROR",
+ )
+
+ def normalize_query_result(self, result: Any, form_data: Mapping[str,
Any]) -> Any:
+ """Reject results that cannot size a Bullet chart without guessing."""
+ from superset.mcp_service.chart.query_result import query_result_data
+
+ data, failure = query_result_data(result, temporal_json_numbers=True)
+ if failure is not None:
+ return failure
+ rows = data[0] if data else []
+ if (error := _render_model_error(rows, form_data)) is not None:
+ return error
+ return result
+
+ def sanitize_data_rows(
+ self, data: list[Any], form_data: Mapping[str, Any]
+ ) -> tuple[list[Any], ChartError | None]:
+ """Expose rows through the same strict model the renderers use."""
+ from superset.mcp_service.chart.preview_utils import (
+ _safe_enum_backing,
+ BulletOutputError,
+ resolve_bullet_render_model,
+ )
+ from superset.mcp_service.chart.query_result import
safe_exception_message
+
+ try:
+ model = resolve_bullet_render_model(data, dict(form_data))
+ except BulletOutputError as ex:
+ return [], ChartError(
+ error=safe_exception_message(ex), error_type=ex.error_type
+ )
+ if not data:
+ # The strict model's zero-valued ungrouped row is a render-only
+ # frontend fallback, not source query data, so an empty query
+ # exposes no rows to get-data and exports.
+ return [], None
+ # The strict model retains exact result keys while replacing unselected
+ # values with None and normalizing the dimensions. Its float measure is
+ # render-only: exported rows keep the validated source metric so exact
+ # BIGINT/Decimal values are not rounded to binary64.
+ rows: list[Any] = []
+ for source, row in zip(data, model.rows, strict=False):
+ exposed = dict(row)
+ if model.metric_field in source:
+ exposed[model.metric_field] = _safe_enum_backing(
+ source[model.metric_field]
+ )
+ rows.append(exposed)
+ return rows, None
+
+ def ascii_preview(
+ self, data: list[Any], form_data: dict[str, Any], width: int
+ ) -> str | ChartError | None:
+ from superset.mcp_service.chart.preview_utils import (
+ _generate_ascii_bullet_chart,
+ BulletOutputError,
+ )
+ from superset.mcp_service.chart.query_result import
safe_exception_message
+
+ try:
+ return _generate_ascii_bullet_chart(data, form_data)
+ except BulletOutputError as ex:
+ return ChartError(
+ error=safe_exception_message(ex), error_type=ex.error_type
+ )
+
+ def vega_lite_preview(
+ self, data: list[Any], form_data: dict[str, Any]
+ ) -> VegaLitePreview | ChartError | None:
+ from superset.mcp_service.chart.preview_utils import (
+ _generate_bullet_vega_lite_preview,
+ BulletOutputError,
+ )
+ from superset.mcp_service.chart.query_result import
safe_exception_message
+
+ try:
+ return _generate_bullet_vega_lite_preview(data, form_data)
+ except BulletOutputError as ex:
+ return ChartError(
+ error=safe_exception_message(ex), error_type=ex.error_type
+ )
+
+ def resolve_update_config(
+ self,
+ config: Any,
+ existing_form_data: dict[str, Any],
+ *,
+ dataset_rebind: bool,
+ ) -> Any:
+ """Resolve sort targets against the saved hierarchy before mapping."""
+ if not isinstance(config, BulletChartConfig) or config.dimensions is
not None:
+ return config
+ if not config.order_by:
+ return config
+ dimensions = (
+ existing_form_data.get("groupby") or []
+ if not dataset_rebind and existing_form_data.get("viz_type") ==
"bullet"
+ else []
+ )
+ return BulletChartConfig.model_validate(
+ {**config.model_dump(exclude_unset=True), "dimensions": dimensions}
+ )
+
+ def merge_update_form_data(
+ self,
+ existing_form_data: dict[str, Any],
+ new_form_data: dict[str, Any],
+ config: Any,
+ *,
+ dataset_rebind: bool,
+ ) -> dict[str, Any] | None:
+ """Merge filter provenance and preserve omitted native Bullet
controls."""
+ if not isinstance(config, BulletChartConfig) or dataset_rebind:
+ return None
+ from superset.mcp_service.chart.chart_utils import (
+ merge_bullet_form_data,
+ merge_update_form_data,
+ )
+
+ merged = dict(new_form_data)
+ merge_update_form_data(existing_form_data, merged, config)
+ merge_bullet_form_data(existing_form_data, merged)
Review Comment:
A presentation-only update of a saved Bullet drops `url_params` here because
this merge starts from mapped controls and skips generic same-viz preservation.
A saved SQL metric using `url_param("region", "US")` with chart metadata
selecting `EU` can therefore silently query `US` after changing labels; could
omitted query metadata survive same-dataset updates?
##########
superset/mcp_service/chart/chart_utils.py:
##########
@@ -1715,6 +1781,668 @@ 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 = {
+ "groupby",
+ "adhoc_filters",
+ "time_range",
+ "row_limit",
+ "orderby",
+ "ranges",
+ "range_labels",
+ "markers",
+ "marker_labels",
+ "marker_lines",
+ "marker_line_labels",
+ "y_axis_format",
+ "show_labels",
+ "show_legend",
+ MCP_DASHBOARD_TIME_FILTER_SUBJECT,
+ }
+
+ # Threshold and label arrays are one frontend control pair. If callers
+ # replace the values without replacing their labels, clear the stale labels
+ # instead of accidentally reassigning them by position.
+ dependent_controls = {
+ "ranges": "range_labels",
+ "markers": "marker_labels",
+ "marker_lines": "marker_line_labels",
+ }
+ for values_key, labels_key in dependent_controls.items():
+ if values_key in new_form_data and labels_key not in new_form_data:
+ new_form_data[labels_key] = ""
+
+ for key in preserved_keys:
+ if (
+ key == MCP_DASHBOARD_TIME_FILTER_SUBJECT
+ and "adhoc_filters" in new_form_data
+ ):
+ # The marker describes a mapper-generated temporal filter. Do not
+ # retain stale provenance when an explicit filter update removed
it.
+ continue
+ if key in existing_form_data and key not in new_form_data:
+ new_form_data[key] = existing_form_data[key]
Review Comment:
The new pruning still retains a sort on `SavedRevenue` when the update
replaces it with `SUM(Revenue)`, so merged validation rejects the
dimension-clear request (the new success test mocks that validation away);
case-folding also leaves a stale sort when only one of `Region` and `region` is
removed. Could pruning use the final exact output roles, with merged validation
enabled in these regressions?
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -496,6 +1091,350 @@ def _is_nan(value: Any) -> bool:
return False
+def _bullet_numeric_tokens(value: Any) -> list[float]:
+ """Parse native comma-separated Bullet threshold controls."""
+ if isinstance(value, str):
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif isinstance(value, list):
+ tokens = value
+ else:
+ return []
+ result: list[float] = []
+ for token in tokens:
+ try:
+ number = float(token)
+ except (TypeError, ValueError):
+ continue
+ if not _is_nan(number) and math.isfinite(number):
+ result.append(number)
+ return result
+
+
+def _bullet_numeric_control_tokens(value: Any, role: str) -> list[float]: #
noqa: C901
+ """Drop non-numeric native tokens like Explore, retaining safety bounds."""
+ value = _safe_enum_backing(value)
+ if value is None or (type(value) is str and value == ""):
+ return []
+ if type(value) is str:
+ if len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(f"Bullet {role} exceeds the size limit")
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif type(value) is list:
+ tokens = [
+ list.__getitem__(value, index) for index in
range(list.__len__(value))
+ ]
+ else:
+ raise BulletOutputError(f"Bullet {role} must be a comma-separated
list")
+ if len(tokens) > _MAX_BULLET_TOKENS:
+ raise BulletOutputError(f"Bullet {role} exceeds the item limit")
+
+ numbers: list[float] = []
+ for index, token in enumerate(tokens):
+ token = _safe_enum_backing(token)
+ if type(token) is str and token == "":
+ continue
+ if type(token) is bool or not (
+ type(token) is str
+ or type(token) is int
+ or type(token) is float
+ or type(token) is Decimal
+ ):
+ raise BulletOutputError(f"Bullet {role}[{index}] is not numeric")
+ if type(token) is str and len(token) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(f"Bullet {role}[{index}] is not numeric")
+ try:
+ number = float(token)
Review Comment:
Native Bullet controls use JavaScript `Number`, so a saved marker `"0x64"`
renders at 100 in Explore but is dropped here, while `"1_000"` is ignored in
Explore but becomes 1,000 here. Could ranges and marker parsing preserve the
frontend tokenizer semantics?
##########
superset/mcp_service/chart/preview_utils.py:
##########
@@ -496,6 +1091,350 @@ def _is_nan(value: Any) -> bool:
return False
+def _bullet_numeric_tokens(value: Any) -> list[float]:
+ """Parse native comma-separated Bullet threshold controls."""
+ if isinstance(value, str):
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif isinstance(value, list):
+ tokens = value
+ else:
+ return []
+ result: list[float] = []
+ for token in tokens:
+ try:
+ number = float(token)
+ except (TypeError, ValueError):
+ continue
+ if not _is_nan(number) and math.isfinite(number):
+ result.append(number)
+ return result
+
+
+def _bullet_numeric_control_tokens(value: Any, role: str) -> list[float]: #
noqa: C901
+ """Drop non-numeric native tokens like Explore, retaining safety bounds."""
+ value = _safe_enum_backing(value)
+ if value is None or (type(value) is str and value == ""):
+ return []
+ if type(value) is str:
+ if len(value) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(f"Bullet {role} exceeds the size limit")
+ tokens: list[Any] = [token.strip() for token in value.split(",")]
+ elif type(value) is list:
+ tokens = [
+ list.__getitem__(value, index) for index in
range(list.__len__(value))
+ ]
+ else:
+ raise BulletOutputError(f"Bullet {role} must be a comma-separated
list")
+ if len(tokens) > _MAX_BULLET_TOKENS:
+ raise BulletOutputError(f"Bullet {role} exceeds the item limit")
+
+ numbers: list[float] = []
+ for index, token in enumerate(tokens):
+ token = _safe_enum_backing(token)
+ if type(token) is str and token == "":
+ continue
+ if type(token) is bool or not (
+ type(token) is str
+ or type(token) is int
+ or type(token) is float
+ or type(token) is Decimal
+ ):
+ raise BulletOutputError(f"Bullet {role}[{index}] is not numeric")
+ if type(token) is str and len(token) > _MAX_BULLET_TEXT_BYTES:
+ raise BulletOutputError(f"Bullet {role}[{index}] is not numeric")
+ try:
+ number = float(token)
+ except ValueError:
+ # Native controls tolerate stray text and incomplete input.
+ continue
+ except (TypeError, OverflowError) as ex:
+ raise BulletOutputError(f"Bullet {role}[{index}] is not numeric")
from ex
+ if math.isnan(number):
+ continue
+ if not math.isfinite(number):
+ raise BulletOutputError(f"Bullet {role}[{index}] is NaN or
infinite")
+ numbers.append(number)
+ return numbers
+
+
+def _generate_bullet_vega_lite_preview( # noqa: C901
+ data: List[Dict[str, Any]], form_data: Dict[str, Any]
+) -> VegaLitePreview:
+ """Build a horizontal layered preview from the shared strict model."""
+ model = resolve_bullet_render_model(data, form_data)
+
+ category_field = _unique_bullet_category_field(model.rows)
+ row_field = _unique_bullet_derived_field(
+ model.rows, "__mcp_bullet_row", (category_field,)
+ )
+ containing_range_labels = (
+ [
+ _containing_bullet_range_label(measure, model.ranges,
model.range_labels)
+ for measure in model.measures
+ ]
+ if model.range_labels
+ else [None] * len(model.rows)
+ )
+ range_tooltip_field = (
+ _unique_bullet_derived_field(
+ model.rows, "__mcp_bullet_range", (category_field, row_field)
+ )
+ if any(label is not None for label in containing_range_labels)
+ else None
+ )
+ values = []
+ for row_index, row in enumerate(model.rows):
+ copied = dict.copy(row)
+ copied[row_field] = row_index
+ copied[category_field] = (
+ ", ".join(
+ _bullet_category_value(dict.get(row, field), field,
row_index)[1]
+ for field in model.dimensions
+ )
+ if model.dimensions
+ else ""
+ )
+ if (
+ range_tooltip_field is not None
+ and containing_range_labels[row_index] is not None
+ ):
+ copied[range_tooltip_field] = containing_range_labels[row_index]
+ values.append(copied)
+
+ # Explore uses indexed rows even when their display labels are identical.
+ category_labels = [row[category_field] for row in values]
+ vega_format = {
+ "SMART_NUMBER": "~s",
+ "SMART_NUMBER_SIGNED": "+~s",
+ }.get(model.y_axis_format, model.y_axis_format)
+ y_encoding = {
+ "field": row_field,
+ "type": "nominal",
+ "title": ", ".join(model.dimensions) if model.dimensions else None,
+ "sort": None,
+ "axis": {"labelExpr": f"{json.dumps(category_labels)}[datum.value]"},
+ }
+ tooltip = [
+ {
+ "field": category_field,
+ "type": "nominal",
+ "title": ", ".join(model.dimensions) if model.dimensions else None,
+ },
+ {
+ "field": model.metric_field,
+ "type": "quantitative",
+ "format": vega_format,
+ },
+ ]
+ if range_tooltip_field is not None:
+ tooltip.append(
+ {"field": range_tooltip_field, "type": "nominal", "title": "Range"}
+ )
+ axis_min = min(
+ 0.0,
+ *model.measures,
+ *model.ranges,
+ *model.markers,
+ *model.marker_lines,
+ )
+ axis_max = max(
+ *model.measures,
+ *model.ranges,
+ *model.markers,
+ *model.marker_lines,
+ )
+ if axis_min == axis_max:
+ axis_max = axis_min + (abs(axis_min) or 1)
+
+ def label_at(labels: list[str], index: int, value: float, prefix: str) ->
str:
+ if index < len(labels) and labels[index]:
+ return labels[index]
+ return (
+ ""
+ if prefix == "Range"
+ else _format_bullet_number(model.y_axis_format, value)
+ )
+
+ def legend_color(name: str) -> dict[str, Any]:
+ return {
+ "datum": name,
+ "type": "nominal",
+ "legend": {"title": None} if model.show_legend else None,
+ }
+
+ layers: list[dict[str, Any]] = []
+ range_entries = sorted(
+ [
+ (
+ threshold,
+ label_at(model.range_labels, index, threshold, "Range"),
+ )
+ for index, threshold in enumerate(model.ranges)
+ ],
+ key=lambda entry: entry[0],
+ reverse=True,
+ )
+ for index, (threshold, label) in enumerate(range_entries):
+ layers.append(
+ {
+ "mark": {
+ "type": "rect",
+ "opacity": max(0.08, 0.28 - index * 0.04),
+ },
+ "encoding": {
+ "x": {"datum": axis_min, "type": "quantitative"},
+ "x2": {"datum": threshold},
+ "y": y_encoding,
+ "color": legend_color(
+ (f"{label}: " if label else "")
+ + f"≤ {_format_bullet_number(model.y_axis_format,
threshold)}"
+ ),
+ "tooltip": [
+ {"value": label, "title": "Range"},
+ {
+ "value": _format_bullet_number(
+ model.y_axis_format, threshold
+ ),
+ "title": "Threshold",
+ },
+ ],
+ },
+ }
+ )
+ if model.show_labels and label:
+ layers.append(
+ {
+ "mark": {"type": "text", "align": "right", "dx": -3},
+ "encoding": {
+ "x": {"datum": threshold, "type": "quantitative"},
+ "y": y_encoding,
+ "text": {"value": label},
+ },
+ }
+ )
+ layers.append(
+ {
+ "mark": {"type": "bar", "tooltip": True, "size": 16},
+ "encoding": {
+ "x": {
+ "field": model.metric_field,
Review Comment:
A metric labeled `Revenue.total` keeps that literal key in the returned
rows, but this Vega field reference treats the dot as a nested lookup, so the
measure and tooltip disappear even though Explore renders them. Could metric
field references escape dots/brackets or use a safe derived alias?
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -615,75 +1112,455 @@ def require_column(value: Any, field_name: str) -> Any:
start_time = require_column(form_data.get("start_time"), "start_time")
end_time = require_column(form_data.get("end_time"), "end_time")
category = require_column(form_data.get("y_axis"), "y_axis")
-
raw_series = form_data.get("series")
series_columns = (
[require_column(raw_series, "series")] if raw_series is not None else
[]
)
-
- raw_tooltip_columns = form_data.get("tooltip_columns") or []
- raw_tooltip_metrics = form_data.get("tooltip_metrics") or []
- if not isinstance(raw_tooltip_columns, list) or len(raw_tooltip_columns) >
50:
+ raw_tooltips = form_data.get("tooltip_columns") or []
+ raw_metrics = form_data.get("tooltip_metrics") or []
+ if not isinstance(raw_tooltips, list) or len(raw_tooltips) > 50:
raise ValueError("Gantt tooltip_columns must contain at most 50
entries")
- if not isinstance(raw_tooltip_metrics, list) or len(raw_tooltip_metrics) >
50:
+ if not isinstance(raw_metrics, list) or len(raw_metrics) > 50:
raise ValueError("Gantt tooltip_metrics must contain at most 50
entries")
tooltip_columns = [
require_column(column, f"tooltip_columns[{index}]")
- for index, column in enumerate(raw_tooltip_columns)
+ for index, column in enumerate(raw_tooltips)
]
+ orderby = _parse_orderby(form_data.get("order_by_cols"))
+ columns = _dedupe_query_fields(
+ [
+ start_time,
+ end_time,
+ category,
+ *series_columns,
+ *tooltip_columns,
+ *(item[0] for item in orderby),
+ ],
+ _column_label,
+ )
+ return columns, list(raw_metrics), orderby, series_columns
- raw_order = form_data.get("order_by_cols") or []
- if not isinstance(raw_order, list) or len(raw_order) > 100:
- raise ValueError("Gantt order_by_cols must contain at most 100
entries")
- orderby: list[list[Any]] = []
- for index, entry in enumerate(raw_order):
- if isinstance(entry, str):
- if len(entry) > 1000:
- raise ValueError(f"Gantt order_by_cols[{index}] is too long")
- try:
- entry = utils_json.loads(entry)
- except (TypeError, ValueError) as ex:
- raise ValueError(
- f"Gantt order_by_cols[{index}] is not valid JSON"
- ) from ex
+
+def _table_time_offsets(form_data: dict[str, Any], query: dict[str, Any]) ->
list[Any]:
+ """Resolve the Table plugin's custom/inherit comparison offsets."""
+ if not _time_comparison(form_data, query.get("metrics") or []):
+ return []
+ offsets: list[Any] = []
+ for offset in _as_list(form_data.get("time_compare")):
+ if offset == "custom":
+ offset = form_data.get("start_date_offset")
+ elif offset == "inherit":
+ offset = "inherit"
+ if offset is not None and offset not in offsets:
+ offsets.append(offset)
+ extra = form_data.get("extra_form_data")
+ if isinstance(extra, dict):
+ offset = extra.get("time_compare")
+ if offset is not None and offset not in offsets:
+ offsets = [offset]
+ return offsets
+
+
+def _table_totals_metrics(metrics: list[Any], aggregate: Any) -> list[Any]:
+ """Mirror ``getTotalsMetrics`` for Table summary queries."""
+ if aggregate not in {"SUM", "AVG"}:
+ return metrics
+ result: list[Any] = []
+ for metric in metrics:
+ if isinstance(metric, dict) and metric.get("expressionType") ==
"SIMPLE":
+ result.append({**metric, "aggregate": aggregate})
+ else:
+ result.append(metric)
+ return result
+
+
+def _temporal_column(column: Any, form_data: dict[str, Any]) -> Any:
+ """Apply the frontend BASE_AXIS wrapper for a physical temporal column."""
+ if not isinstance(column, str) or not form_data.get("time_grain_sqla"):
+ return column
+ lookup = form_data.get("temporal_columns_lookup")
+ if not isinstance(lookup, dict) or not lookup.get(column):
+ return column
+ return {
+ "timeGrain": form_data["time_grain_sqla"],
+ "columnType": "BASE_AXIS",
+ "sqlExpression": column,
+ "label": column,
+ "expressionType": "SQL",
+ }
+
+
+def _normalize_orderby(query: dict[str, Any]) -> None:
+ """Mirror ``normalizeOrderBy`` without dropping independent mixed state."""
+ orderby = query.get("orderby")
+ if (
+ isinstance(orderby, list)
+ and orderby
+ and isinstance(orderby[0], (list, tuple))
+ and len(orderby[0]) == 2
+ and orderby[0][0]
+ and isinstance(orderby[0][1], bool)
+ ):
+ return
+ query.pop("orderby", None)
+ target = query.get("series_limit_metric") or query.get("legacy_order_by")
+ if target is None:
+ metrics = query.get("metrics") or []
+ target = metrics[0] if metrics else None
+ if target is not None:
+ query["orderby"] = [[target, not query.get("order_desc", True)]]
+
+
+def _time_comparison(form_data: dict[str, Any], metrics: list[Any]) -> bool:
+ return bool(
+ metrics
+ and _as_list(form_data.get("time_compare"))
+ and form_data.get("comparison_type")
+ in {"values", "difference", "percentage", "ratio"}
+ )
+
+
+def _timeseries_post_processing( # noqa: C901
+ form_data: dict[str, Any],
+ query: dict[str, Any],
+ *,
+ operator_metrics: list[Any] | None = None,
+ complete_timeseries_contract: bool = False,
+) -> list[dict[str, Any]]:
+ """Build the frontend Mixed/Timeseries post-processing contract.
+
+ Timeseries passes its pre-extra-metric QueryObject to every operator, while
+ adding ``extractExtraMetrics`` only to its final query and normal pivot.
+ Mixed passes each layer QueryObject and implements the smaller operator set
+ in its own frontend builder.
+ """
+ metrics = (
+ list(operator_metrics)
+ if operator_metrics is not None
+ else list(query.get("metrics") or [])
+ )
+ metric_labels = [label for metric in metrics if (label :=
_metric_label(metric))]
+ x_axis = form_data.get("x_axis")
+ x_label = (
+ _column_label(x_axis)
+ if x_axis
+ else ("__timestamp" if form_data.get("granularity_sqla") else None)
+ )
+ series = _query_series_columns(query)
+ series_labels = [label for column in series if (label :=
_column_label(column))]
+ offsets = _as_list(form_data.get("time_compare"))
+ comparison = _time_comparison(form_data, metrics)
+ offset_map = {
+ f"{metric}__{offset}": metric for metric in metric_labels for offset
in offsets
+ }
+ pivot_metrics = (
+ [*offset_map.values(), *offset_map.keys()]
+ if comparison
+ else [
+ *metric_labels,
+ *(
+ [
+ label
+ for metric in _timeseries_extra_metrics(form_data)
+ if (label := _metric_label(metric))
+ ]
+ if complete_timeseries_contract
+ else []
+ ),
+ ]
+ )
+ chain: list[dict[str, Any] | None] = []
+ if x_label and pivot_metrics:
+ chain.append(
+ {
+ "operation": "pivot",
+ "options": {
+ "index": [x_label],
+ "columns": series_labels,
+ "aggregates": {
+ metric: {"operator": "mean"} for metric in
pivot_metrics
+ },
+ "drop_missing_columns": not form_data.get(
+ "show_empty_columns", False
+ ),
+ },
+ }
+ )
+ method = form_data.get("resample_method")
+ rule = form_data.get("resample_rule")
+ if method and rule:
+ zero_fill = method == "zerofill"
+ chain.append(
+ {
+ "operation": "resample",
+ "options": {
+ "method": "asfreq" if zero_fill else method,
+ "rule": rule,
+ "fill_value": 0 if zero_fill else None,
+ },
+ }
+ )
+ rolling_type = form_data.get("rolling_type")
+ rolling_columns = (
+ [*offset_map.values(), *offset_map.keys()] if comparison else
metric_labels
+ )
+ if rolling_type == "cumsum":
+ chain.append(
+ {
+ "operation": "cum",
+ "options": {
+ "operator": "sum",
+ "columns": {column: column for column in rolling_columns},
+ },
+ }
+ )
+ elif rolling_type in {"sum", "mean", "std"}:
+ chain.append(
+ {
+ "operation": "rolling",
+ "options": {
+ "rolling_type": rolling_type,
+ "window": int(form_data.get("rolling_periods") or 1),
+ "min_periods": int(form_data.get("min_periods") or 0),
+ "columns": {column: column for column in rolling_columns},
+ },
+ }
+ )
+ comparison_type = form_data.get("comparison_type")
+ if comparison and comparison_type != "values":
+ chain.append(
+ {
+ "operation": "compare",
+ "options": {
+ "source_columns": list(offset_map.values()),
+ "compare_columns": list(offset_map.keys()),
+ "compare_type": comparison_type,
+ "drop_original_columns": True,
+ },
+ }
+ )
+ if complete_timeseries_contract and form_data.get("contributionMode"):
+ chain.append(
+ {
+ "operation": "contribution",
+ "options": {
+ "orientation": form_data["contributionMode"],
+ "time_shifts": offsets if comparison else [],
+ },
+ }
+ )
+ if comparison:
+ rename: dict[str, str | None] = {}
+ for shifted, metric in offset_map.items():
+ offset = next(
+ (item for item in offsets if shifted.endswith(f"__{item}")),
None
+ )
+ source = (
+ shifted
+ if comparison_type == "values"
+ else f"{comparison_type}__{metric}__{shifted}"
+ )
+ rename[source] = f"{metric}, {offset}" if len(metrics) > 1 else
offset
+ if rename:
+ chain.append(
+ {
+ "operation": "rename",
+ "options": {"columns": rename, "level": 0, "inplace":
True},
+ }
+ )
+ elif (
+ x_label
+ and len(metrics) == 1
+ and (series_labels or len(offsets) > 1)
+ and form_data.get("truncate_metric") is not None
+ and form_data.get("truncate_metric")
+ ):
+ chain.append(
+ {
+ "operation": "rename",
+ "options": {
+ "columns": {metric_labels[0]: None},
+ "level": 0,
+ "inplace": True,
+ },
+ }
+ )
+ if complete_timeseries_contract:
+ x_axis_sort = form_data.get("x_axis_sort")
+ x_axis_sort_asc = form_data.get("x_axis_sort_asc")
+ sortable_labels = [
+ label
+ for label in [
+ x_label,
+ *(
+ _metric_label(metric)
+ for metric in _as_list(form_data.get("metrics"))
+ ),
+ *(
+ _metric_label(metric)
+ for metric in _timeseries_extra_metrics(form_data)
+ ),
+ ]
+ if label
+ ]
if (
- not isinstance(entry, (list, tuple))
- or len(entry) != 2
- or not isinstance(entry[0], str)
- or not entry[0]
- or not isinstance(entry[1], bool)
+ x_axis_sort is not None
+ and x_axis_sort_asc is not None
+ and x_axis_sort in sortable_labels
+ and not _as_list(form_data.get("groupby"))
):
- raise ValueError(
- f"Gantt order_by_cols[{index}] must be [column,
ascending_boolean]"
- )
- orderby.append([entry[0], entry[1]])
+ options: dict[str, Any] = {"ascending": x_axis_sort_asc}
+ if x_axis_sort == x_label:
+ options["is_sort_index"] = True
+ else:
+ options["by"] = x_axis_sort
+ chain.append({"operation": "sort", "options": options})
+ chain.append({"operation": "flatten"})
+
+ if complete_timeseries_contract and form_data.get("forecastEnabled") and
x_label:
+ x_axis_grain = (
+ x_axis.get("timeGrain")
+ if isinstance(x_axis, dict)
+ and x_axis.get("expressionType") in {"SIMPLE", "SQL"}
+ else None
+ )
+ time_grain = (
+ x_axis_grain
+ or (query.get("extras") or {}).get("time_grain_sqla")
+ or form_data.get("time_grain_sqla")
+ or "P1D"
+ )
+ chain.append(
+ {
+ "operation": "prophet",
+ "options": {
+ "time_grain": time_grain,
+ "periods": int(form_data.get("forecastPeriods", 10)),
+ "confidence_interval": float(
+ form_data.get("forecastInterval", 0.8)
+ ),
+ "yearly_seasonality":
form_data.get("forecastSeasonalityYearly"),
+ "weekly_seasonality":
form_data.get("forecastSeasonalityWeekly"),
+ "daily_seasonality":
form_data.get("forecastSeasonalityDaily"),
+ "index": x_label,
+ },
+ }
+ )
+ return [operator for operator in chain if operator is not None]
- columns: list[Any] = []
- seen: set[str] = set()
- for column in (
- start_time,
- end_time,
- category,
- *series_columns,
- *tooltip_columns,
- *(entry[0] for entry in orderby),
+
+def _mixed_layer_form_data(
+ form_data: dict[str, Any], *, secondary: bool
+) -> dict[str, Any]:
+ """Mirror MixedTimeseries remove/retainFormDataSuffix for one layer."""
+ if not secondary:
+ return {
+ key: value for key, value in form_data.items() if not
key.endswith("_b")
+ }
+ layer = {key: value for key, value in form_data.items() if not
key.endswith("_b")}
+ for isolated_key in (
+ "metrics",
+ "groupby",
+ "orderby",
+ "limit",
+ "series_limit",
+ "timeseries_limit_metric",
+ "series_limit_metric",
+ "order_desc",
+ "row_limit",
+ "truncate_metric",
+ "time_compare",
+ "comparison_type",
+ "resample_method",
+ "resample_rule",
+ "rolling_type",
+ "rolling_periods",
+ "min_periods",
+ "show_empty_columns",
):
- key = utils_json.dumps(column, sort_keys=True, default=str)
- if key not in seen:
- seen.add(key)
- columns.append(column)
- return columns, list(raw_tooltip_metrics), orderby, series_columns
+ if f"{isolated_key}_b" not in form_data:
Review Comment:
For a Mixed chart with shared `row_limit: 25` and no `row_limit_b`, this
removes the limit from query B, while the frontend retains the shared limit
when there is no suffixed override. `get_chart_sql` can therefore describe a
differently bounded secondary query from Explore; could shared limits survive
this layer isolation?
##########
superset/mcp_service/chart/compile.py:
##########
@@ -282,6 +329,431 @@ 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":
+ 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 = [
+ 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_lookup = form_data.get("temporal_columns_lookup")
+ if isinstance(temporal_lookup, dict):
+ for column, enabled in temporal_lookup.items():
+ if enabled and (error := column_error(column, "temporal lookup
column")):
Review Comment:
`temporal_columns_lookup` records all temporal columns in the old
datasource, including ones the chart never selects. A Table rebind with valid
replacement roles still fails here if the new dataset lacks one of those unused
dates, because pruning and same-viz merging retain the lookup; could this
validate only selected roles or rebuild that metadata for the replacement
dataset?
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -1338,6 +1340,51 @@ 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.
+
+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.
+
+Range, marker, and marker-line label lists may be shorter than their value
lists.
+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.
+
+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.
+
+### Chart query result limits
+
+MCP chart tools accept up to 50,000 rows per query. The row-shaped `indexnames`
Review Comment:
A one-row query containing a 65,537-byte text cell now fails with
`MalformedQueryResult` regardless of the configured response-size guard, but
this limits section documents neither the new 64 KiB cell cap nor the 16 MiB
aggregate cap. Could the guide and upgrade notes describe these hard limits and
the affected query/export paths?
##########
superset/mcp_service/chart/query_result.py:
##########
@@ -18,90 +18,1875 @@
"""Helpers for interpreting ChartDataCommand result envelopes."""
import math
-from collections.abc import Mapping
+import time as system_time
+from bisect import bisect_right
+from collections.abc import Mapping, Sequence
+from dataclasses import dataclass
+from datetime import date, datetime, time, timedelta, timezone
from decimal import Decimal
+from enum import Enum
from numbers import Real
+from types import MappingProxyType
from typing import Any, cast
+from uuid import UUID
+from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
+
+import numpy as np
+import pandas as pd
+import pytz
+from dateutil import tz as dateutil_tz
+from dateutil.tz.tz import _ttinfo as dateutil_ttinfo
+from dateutil.zoneinfo import tzfile as dateutil_zoneinfo_tzfile
+from pydantic import BaseModel
+from pydantic_core import to_json
from superset.mcp_service.chart.schemas import ChartError
+from superset.mcp_service.utils.serialization import decode_binary
+from superset.utils.core import GenericDataType
+from superset.utils.dates import datetime_to_epoch, EPOCH
FAILED_QUERY_STATUSES = frozenset(
{"error", "failed", "stopped", "timed_out", "cancelled", "canceled"}
)
+_ERROR_KEYS = ("error", "error_message", "message", "detail")
+_MAX_ERROR_DEPTH = 32
+_MAX_ERROR_ITEMS = 256
+_MAX_SEQUENCE_ITEMS = 64
+_MAX_ERROR_PARTS = 3
+_MAX_ERROR_BYTES = 2000
+_MAX_INTEGER_DIGITS = 1000
+_MAX_QUERY_COUNT = 64
+_MAX_QUERY_COLUMNS = 4096
+_MAX_COLUMN_NAME_BYTES = 4096
+_MAX_ROW_CONTAINER_DEPTH = 32
+_MAX_ROW_CONTAINER_ITEMS = 4096
+_MAX_CACHE_STRING_BYTES = 4096
+_MAX_RESULT_ROW_COUNT = (1 << 63) - 1
-def _query_error_text(value: Any) -> str | None:
- """Convert a bounded query error payload into a useful message."""
- if value is None or value is False:
+# Chart results are routinely much larger than an MCP response should return,
but
+# legitimate exports and high-cardinality chart queries still need useful room.
+# Each query may return Superset's configured 50k ROW_LIMIT. The aggregate row
+# budget admits both legs of Big Number raw/trend and Mixed Timeseries results
+# at that limit, while the value budget admits twenty scalar columns on both
+# legs (plus their row containers). The complete compact JSON projection is
+# capped at 16 MiB, including scalar tokens, escaping, keys, and syntax.
Metadata
+# profiling has a separate row-by-column work budget in ``response_utils`` so
+# wide sparse results cannot turn bounded validation into an unbounded scan.
+# Individual source-result cell strings are capped at 64 KiB and object keys at
+# 4 KiB. Derived strings in a final Pydantic response have no per-cell cap; the
+# complete compact response remains subject to the 16 MiB aggregate budget.
+# Query metadata has its own 1 MiB aggregate budget so SQL and cache metadata
+# cannot consume the row-data allowance. Row-shaped indexnames use the row-data
+# work budget while retaining the metadata byte budget. Integer/Decimal bounds
+# prevent later hashing, uniqueness, and JSON conversion from allocating by
magnitude.
+MAX_QUERY_RESULT_ROWS = 50_000
+MAX_QUERY_RESULT_TOTAL_ROWS = 2 * MAX_QUERY_RESULT_ROWS
+MAX_QUERY_RESULT_VALUES = 2_500_000
+MAX_QUERY_RESULT_VALUE_BYTES = 16 * 1024 * 1024
+MAX_QUERY_RESULT_METADATA_BYTES = 1024 * 1024
+MAX_QUERY_RESULT_METADATA_ITEMS = 32_768
+MAX_QUERY_RESULT_WORK = MAX_QUERY_RESULT_VALUES +
MAX_QUERY_RESULT_METADATA_ITEMS
+MAX_QUERY_RESULT_STRING_BYTES = 64 * 1024
+MAX_QUERY_RESULT_KEY_BYTES = 4096
+MAX_QUERY_RESULT_INTEGER_BITS = 4096
+MAX_QUERY_RESULT_INTEGER_DIGITS = 1234
+MAX_QUERY_RESULT_DECIMAL_DIGITS = 1024
+MAX_QUERY_RESULT_DECIMAL_EXPONENT = 4096
+MAX_QUERY_RESULT_DECIMAL_STORAGE = 2048
+_BUILTIN_SCALAR_TYPES = (str, bytes, bytearray, memoryview, int, float, bool)
+_SCALAR_BASE_TYPES = (*_BUILTIN_SCALAR_TYPES, Enum)
+_SUPPORTED_COLTYPES = frozenset(GenericDataType)
+_TRUSTED_TZINFO_TYPES = (timezone, ZoneInfo)
+_DATEUTIL_TZFILE_TYPE = dateutil_tz.tzfile
+_DATEUTIL_TZOFFSET_TYPE = type(dateutil_tz.tzoffset(None, 0))
+_DATEUTIL_TZUTC_TYPE = type(dateutil_tz.UTC)
+_DATEUTIL_TZLOCAL_TYPE = type(dateutil_tz.tzlocal())
+_MAX_DATEUTIL_TRANSITIONS = 4096
+_PYTZ_FIXED_OFFSET_TYPE = type(pytz.FixedOffset(1))
+_PYTZ_UTC_TYPE = type(pytz.UTC)
+_PYTZ_NAMED_BASE_TYPES = (pytz.tzinfo.DstTzInfo, pytz.tzinfo.StaticTzInfo)
+_NUMPY_INTEGER_TYPES = frozenset(
+ type(value)
+ for value in (
+ np.int8(0),
+ np.int16(0),
+ np.int32(0),
+ np.int64(0),
+ np.uint8(0),
+ np.uint16(0),
+ np.uint32(0),
+ np.uint64(0),
+ )
+)
+_NUMPY_FLOAT_TYPES = frozenset(
+ type(value)
+ for value in (np.float16(0), np.float32(0), np.float64(0),
np.longdouble(0))
+)
+_PANDAS_NAT_TYPE = type(pd.NaT)
+_PANDAS_NA_TYPE = type(pd.NA)
+_PANDAS_PERIOD_TYPE = type(pd.Period("2000-01", freq="M"))
+_PANDAS_INTERVAL_TYPE = type(pd.Interval(0, 1))
+
+
+@dataclass(frozen=True)
+class _ErrorText:
+ """Bounded error extraction outcome."""
+
+ text: str | None = None
+ malformed: str | None = None
+
+
+@dataclass
+class _ResultBudget:
+ """Aggregate work counters shared across all queries in one result."""
+
+ rows: int = 0
+ values: int = 0
+ json_bytes: int = 0
+ metadata_items: int = 0
+ metadata_bytes: int = 0
+
+
+def _truncate_utf8(value: str, max_bytes: int) -> str:
+ """Return bounded, replacement-decoded UTF-8 text.
+
+ Encoding even the non-truncated path is intentional: Python strings may
+ contain unpaired surrogates, while MCP/JSON responses must always be valid
+ UTF-8. Slicing by characters before encoding also prevents an
+ attacker-sized string from being encoded in full.
+ """
+ if max_bytes <= 0:
+ return ""
+ candidate = value[:max_bytes]
+ encoded = candidate.encode("utf-8", errors="replace")
+ if len(encoded) <= max_bytes and len(candidate) == len(value):
+ return encoded.decode("utf-8", errors="replace")
+ suffix = "... [truncated]"
+ suffix_bytes = suffix.encode()
+ if max_bytes <= len(suffix_bytes):
+ return suffix_bytes[:max_bytes].decode("ascii")
+ content_limit = max(0, max_bytes - len(suffix_bytes))
+ content = encoded[:content_limit].decode("utf-8", errors="ignore")
+ return content + suffix
+
+
+def _type_descriptor(value: Any, max_bytes: int) -> str | None:
+ """Describe an unsupported value without consulting its implementation."""
+ if max_bytes <= 0:
return None
- if isinstance(value, Mapping):
- for key in ("error", "error_message", "message", "detail"):
- if text := _query_error_text(value.get(key)):
- return text
+ value_type = type(value)
+ try:
+ type_name = type.__getattribute__(value_type, "__name__")
+ except (AttributeError, TypeError): # pragma: no cover - defensive
metaclass
+ type_name = "unknown"
+ if type(type_name) is not str:
+ type_name = "unknown"
+ bounded_name = _truncate_utf8(type_name, max_bytes)
+ return _truncate_utf8(f"<{bounded_name} object>", max_bytes)
+
+
+def _type_mro(value_type: type[Any]) -> tuple[type[Any], ...]:
+ """Read a concrete type's MRO without consulting its metaclass
overrides."""
+ try:
+ mro = type.__getattribute__(value_type, "__mro__")
+ except (AttributeError, TypeError): # pragma: no cover - all normal types
have MRO
+ return ()
+ return mro if type(mro) is tuple else ()
+
+
+def _mro_contains(
+ value_mro: tuple[type[Any], ...], base_types: tuple[type[Any], ...]
+) -> bool:
+ """Return whether an MRO contains a base, using identity-only
comparisons."""
+ return any(
+ base is expected_base for base in value_mro for expected_base in
base_types
+ )
+
+
+def _safe_scalar_text(value: Any, max_bytes: int) -> str | None: # noqa: C901
+ """Render a bounded scalar without invoking attacker-controlled string
code."""
+ value_type = type(value)
+ if _mro_contains(_type_mro(value_type), (Enum,)):
+ try:
+ enum_value = object.__getattribute__(value, "_value_")
+ except Exception:
+ return _type_descriptor(value, max_bytes)
+ if not any(
+ type(enum_value) is scalar_type for scalar_type in
_BUILTIN_SCALAR_TYPES
+ ):
+ return _type_descriptor(value, max_bytes)
+ return _safe_scalar_text(enum_value, max_bytes)
+ if value is None or value is False:
return None
- if isinstance(value, (list, tuple)):
- parts = [text for item in value if (text := _query_error_text(item))]
- return "; ".join(parts[:3]) or None
- text = str(value)
- return text[:2000] if text else None
+ if value_type is str:
+ return _truncate_utf8(value, max_bytes) if value else None
+ if value_type is bytes or value_type is bytearray or value_type is
memoryview:
+ try:
+ view = memoryview(value).cast("B")
+ sample = view[: max(0, max_bytes)].tobytes()
+ text = sample.decode("utf-8", errors="replace")
+ if len(view) > len(sample):
+ text += "... [truncated]"
+ return _truncate_utf8(text, max_bytes) if text else None
+ except (TypeError, ValueError):
+ return _type_descriptor(value, max_bytes)
+ if value_type is int:
+ digits = (
+ 1 if value == 0 else int((abs(value).bit_length() - 1) *
math.log10(2)) + 1
+ )
+ if digits > _MAX_INTEGER_DIGITS:
+ sign = "negative " if value < 0 else ""
+ return _truncate_utf8(
+ f"<{sign}integer with approximately {digits} decimal digits>",
+ max_bytes,
+ )
+ return _truncate_utf8(str(value), max_bytes)
+ if value_type is bool or value_type is float:
+ return _truncate_utf8(str(value), max_bytes)
+ return _type_descriptor(value, max_bytes)
+
+
+def _query_error_text(value: Any) -> _ErrorText: # noqa: C901
+ """Iteratively extract actionable text from an untrusted error payload.
+
+ Chart backends and engine adapters can return arbitrary nested error
shapes.
+ Depth, visited-item, sequence-width, and output-byte limits keep validation
+ deterministic even for cycles, repeated containers, and adversarial values.
+ """
+ stack: list[tuple[Any, int]] = [(value, 0)]
+ seen: set[int] = set()
+ parts: list[str] = []
+ visited = 0
+ used_bytes = 0
+
+ while stack and len(parts) < _MAX_ERROR_PARTS:
+ item, depth = stack.pop()
+ visited += 1
+ if visited > _MAX_ERROR_ITEMS:
+ return _ErrorText(malformed="error payload exceeds the item limit")
+ if depth > _MAX_ERROR_DEPTH:
+ return _ErrorText(malformed="error payload exceeds the depth
limit")
+
+ # ChartDataCommand envelopes cross a JSON boundary. Only exact JSON
+ # containers are trusted here: ABC/isinstance checks can consult a
+ # spoofed ``__class__``, and subclass get/contains/iter/len hooks are
+ # attacker-controlled. Exact dict/list operations below are builtin and
+ # non-overridable.
+ is_mapping = type(item) is dict
+ is_sequence = type(item) is list
+ item_mro = _type_mro(type(item))
+ if not (is_mapping or is_sequence) and (
+ _mro_contains(item_mro, (dict, list, Mapping, Sequence))
+ and not _mro_contains(item_mro, _SCALAR_BASE_TYPES)
+ ):
+ return _ErrorText(
+ malformed="error payload contains an unsupported container
type"
+ )
+ if is_mapping or is_sequence:
+ identity = id(item)
+ if identity in seen:
+ return _ErrorText(
+ malformed="error payload contains repeated or cyclic
containers"
+ )
+ seen.add(identity)
+
+ if is_mapping:
+ children: list[Any] = []
+ for key in _ERROR_KEYS:
+ if dict.__contains__(item, key):
+ children.append(dict.__getitem__(item, key))
+ if not children:
+ if dict.__len__(item):
+ return _ErrorText(
+ malformed=(
+ "error payload object has no recognized message
field"
+ )
+ )
+ stack.extend((child, depth + 1) for child in reversed(children))
+ continue
+
+ if is_sequence:
+ width = list.__len__(item)
+ if width > _MAX_SEQUENCE_ITEMS:
+ return _ErrorText(malformed="error payload exceeds the width
limit")
+ children = [list.__getitem__(item, index) for index in
range(width)]
+ stack.extend((child, depth + 1) for child in reversed(children))
+ continue
+
+ remaining = _MAX_ERROR_BYTES - used_bytes - (2 if parts else 0)
+ text = _safe_scalar_text(item, remaining)
+ if text:
+ parts.append(text)
+ used_bytes += len(text.encode("utf-8", errors="replace")) + (
+ 2 if len(parts) > 1 else 0
+ )
+ return _ErrorText(text="; ".join(parts) or None)
-def _failure_for_query_payload(
- payload: Mapping[str, Any], label: str
+
+def _failure_for_query_payload( # noqa: C901
+ payload: dict[str, Any], label: str
) -> ChartError | None:
"""Extract one failure from a top-level or per-query payload."""
+ malformed: str | None = None
for key in ("error", "errors", "error_message"):
- if message := _query_error_text(payload.get(key)):
+ extracted = _query_error_text(dict.get(payload, key))
+ if extracted.malformed:
+ malformed = malformed or extracted.malformed
+ continue
+ if message := extracted.text:
return ChartError(
error=f"{label} failed: {message}", error_type="QueryError"
)
- raw_status = payload.get("status")
- status = str(getattr(raw_status, "value", raw_status) or "")
+ raw_status = dict.get(payload, "status")
+ status = _safe_scalar_text(raw_status, 200) or ""
normalized_status = status.strip().casefold().replace("-", "_").replace("
", "_")
if normalized_status in FAILED_QUERY_STATUSES:
- message = (
- _query_error_text(payload.get("message"))
- or _query_error_text(payload.get("error_message"))
- or normalized_status
- )
+ extracted = _query_error_text(dict.get(payload, "message"))
+ if extracted.malformed:
+ malformed = malformed or extracted.malformed
+ fallback = _query_error_text(dict.get(payload, "error_message"))
+ if fallback.malformed:
+ malformed = malformed or fallback.malformed
+ if malformed and not (extracted.text or fallback.text):
+ return _malformed_result(malformed)
+ message = extracted.text or fallback.text or normalized_status
return ChartError(error=f"{label} failed: {message}",
error_type="QueryError")
- if payload.get("success") is False:
- message = _query_error_text(payload.get("message")) or "request failed"
+ if dict.get(payload, "success") is False:
+ extracted = _query_error_text(dict.get(payload, "message"))
+ if extracted.malformed:
+ malformed = malformed or extracted.malformed
+ if malformed and not extracted.text:
+ return _malformed_result(malformed)
+ message = extracted.text or "request failed"
return ChartError(error=f"{label} failed: {message}",
error_type="QueryError")
if (
raw_status is None
- and "data" not in payload
- and "queries" not in payload
- and (message := _query_error_text(payload.get("message")))
+ and "data" not in dict.keys(payload)
+ and "queries" not in dict.keys(payload)
):
- return ChartError(error=f"{label} failed: {message}",
error_type="QueryError")
+ extracted = _query_error_text(dict.get(payload, "message"))
+ if extracted.malformed:
+ malformed = malformed or extracted.malformed
+ if extracted.text:
+ return ChartError(
+ error=f"{label} failed: {extracted.text}",
error_type="QueryError"
+ )
+ if malformed:
+ return _malformed_result(malformed)
return None
-def query_result_failure(result: Any) -> ChartError | None:
- """Return a structured failure embedded in a ChartDataCommand payload.
+def _malformed_result(message: str) -> ChartError:
+ """Build a stable error for an invalid ChartDataCommand envelope."""
+ return ChartError(
+ error=f"Malformed chart query result: {message}",
+ error_type="MalformedQueryResult",
+ )
- ChartDataCommand can return an HTTP-successful envelope whose top level or
- any query reports a failure. Every query is inspected before callers accept
- data from the result. Successful statuses may carry informational messages,
- so ``message`` alone is not treated as an error.
+
+def bounded_result_row_count(value: Any) -> int | None:
+ """Return one exact bounded row count, rejecting coercive lookalikes."""
+ if value is None:
+ return None
+ if type(value) is int:
+ count = value
+ elif type(value) is float and math.isfinite(value) and value.is_integer():
+ count = int(value)
+ else:
+ raise ValueError("must be a finite non-negative integral number")
+ if count < 0:
+ raise ValueError("must be non-negative")
+ if count > _MAX_RESULT_ROW_COUNT:
+ raise ValueError("exceeds the supported bound")
+ return count
+
+
+def _bounded_utf8_length(value: str, max_bytes: int) -> int | None:
+ """Return an exact UTF-8 size without encoding attacker-sized text."""
+ if str.__len__(value) > max_bytes:
+ return None
+ try:
+ encoded = str.encode(value, "utf-8", errors="strict")
+ except UnicodeEncodeError:
+ return None
+ size = bytes.__len__(encoded)
+ return size if size <= max_bytes else None
+
+
+def _json_string_size(value: str, max_bytes: int) -> int | None:
+ """Return the exact UTF-8 size of a JSON string without serializing it."""
+ raw_size = _bounded_utf8_length(value, max_bytes)
+ if raw_size is None:
+ return None
+ escaped_size = raw_size + 2 # surrounding quotes
+ for character in value:
+ codepoint = ord(character)
+ if character in {'"', "\\"} or character in {"\b", "\t", "\n", "\f",
"\r"}:
+ escaped_size += 1
+ elif codepoint < 0x20:
+ # Other JSON control characters use a six-byte ``\\u00xx`` escape.
+ escaped_size += 5
+ return escaped_size
+
+
+def _integer_json_size(value: int) -> int:
+ """Return an exact integer JSON size without creating its decimal
string."""
+ magnitude = -value if value < 0 else value
+ if magnitude == 0:
+ digits = 1
+ else:
+ bits = int.bit_length(magnitude)
+ # This fixed-point log10(2) estimate is at most one digit low. Refine
it
+ # with one bounded integer comparison rather than rendering the value.
+ digits = ((bits - 1) * 30103) // 100000 + 1
+ if magnitude >= 10**digits:
+ digits += 1
+ return digits + (value < 0)
+
+
+def _container_json_syntax_size(item_count: int, *, mapping: bool) -> int:
+ """Return braces/brackets, separators, and mapping-colon byte cost."""
+ if item_count == 0:
+ return 2
+ return 2 + item_count - 1 + (item_count if mapping else 0)
+
+
+def _trusted_timedelta_text(value: timedelta) -> str:
+ """Render an exact timedelta with Pydantic's stable ISO-8601 spelling."""
+ total_microseconds = (
+ value.days * 86_400 + value.seconds
+ ) * 1_000_000 + value.microseconds
+ sign = "-" if total_microseconds < 0 else ""
+ remaining = abs(total_microseconds)
+ days, remaining = divmod(remaining, 86_400 * 1_000_000)
+ years, days = divmod(days, 365)
+ hours, remaining = divmod(remaining, 3_600 * 1_000_000)
+ minutes, remaining = divmod(remaining, 60 * 1_000_000)
+ seconds, microseconds = divmod(remaining, 1_000_000)
+
+ date_parts = [f"{years}Y" if years else "", f"{days}D" if days else ""]
+ time_parts = [f"{hours}H" if hours else "", f"{minutes}M" if minutes else
""]
+ if microseconds:
+ fraction = f"{microseconds:06d}".rstrip("0")
+ time_parts.append(f"{seconds}.{fraction}S")
+ elif seconds:
+ time_parts.append(f"{seconds}S")
+
+ date_text = "".join(date_parts)
+ time_text = "".join(time_parts)
+ if not date_text and not time_text:
+ time_text = "0S"
+ return f"{sign}P{date_text}{'T' if time_text else ''}{time_text}"
+
+
+def _chart_data_builtin_timedelta_text(value: timedelta) -> str:
+ """Reproduce ``format_timedelta`` without comparison or string hooks."""
+ total_microseconds = (
+ value.days * 86_400 + value.seconds
+ ) * 1_000_000 + value.microseconds
+ sign = "-" if total_microseconds < 0 else ""
+ remaining = abs(total_microseconds)
+ days, remaining = divmod(remaining, 86_400 * 1_000_000)
+ hours, remaining = divmod(remaining, 3_600 * 1_000_000)
+ minutes, remaining = divmod(remaining, 60 * 1_000_000)
+ seconds, microseconds = divmod(remaining, 1_000_000)
+ day_text = f"{days} {'day' if days == 1 else 'days'}, " if days else ""
+ fraction = f".{microseconds:06d}" if microseconds else ""
+ return f"{sign}{day_text}{hours}:{minutes:02d}:{seconds:02d}{fraction}"
+
+
+def _chart_data_pandas_timedelta_text(value: pd.Timedelta) -> str:
+ """Reproduce Chart Data ``format_timedelta`` output from exact fields."""
+ total_nanoseconds = (
+ (
+ object.__getattribute__(value, "days") * 86_400
+ + object.__getattribute__(value, "seconds")
+ )
+ * 1_000_000
+ + object.__getattribute__(value, "microseconds")
+ ) * 1_000 + object.__getattribute__(value, "nanoseconds")
+ sign = "-" if total_nanoseconds < 0 else ""
+ remaining = abs(total_nanoseconds)
+ days, remaining = divmod(remaining, 86_400 * 1_000_000_000)
+ hours, remaining = divmod(remaining, 3_600 * 1_000_000_000)
+ minutes, remaining = divmod(remaining, 60 * 1_000_000_000)
+ seconds, nanoseconds = divmod(remaining, 1_000_000_000)
+ if nanoseconds % 1_000:
+ fraction = f".{nanoseconds:09d}"
+ elif nanoseconds:
+ fraction = f".{nanoseconds // 1_000:06d}"
+ else:
+ fraction = ""
+ return f"{sign}{days} days
{hours:02d}:{minutes:02d}:{seconds:02d}{fraction}"
+
+
+def _normalized_scalar_json_size( # noqa: C901
+ value: Any, *, max_string_bytes: int = MAX_QUERY_RESULT_STRING_BYTES
+) -> int:
+ """Return a conservative encoded size for one normalized exact scalar."""
+ value_type = type(value)
+ if value is None:
+ return 4
+ if value_type is bool:
+ return 4 if value else 5
+ if value_type is str:
+ size = _json_string_size(value, max_string_bytes)
+ assert size is not None # scalar normalization already bounded the
string
+ return size
+ if value_type is int:
+ return _integer_json_size(value)
+ if value_type is float:
+ if not math.isfinite(value):
+ # Raw Gauge exports retain these markers; JSON responses use null.
+ return 4
+ # Exact builtin repr is hook-free, bounded to a shortest-round-trip
+ # spelling, and avoids pessimistically charging 24 bytes for values
+ # such as 0.0 across ordinary large numeric datasets.
+ return len(float.__repr__(value))
+ if value_type is Decimal:
+ # Decimal storage, coefficient digits, and exponent are bounded before
+ # this point. Its canonical spelling is therefore itself bounded, and
+ # Pydantic serializes Decimal values as JSON strings.
+ text = Decimal.__str__(value)
+ size = _json_string_size(text, MAX_QUERY_RESULT_STRING_BYTES)
+ assert size is not None
+ return size
+ if value_type is datetime:
+ return 40
+ if value_type is date:
+ text = date.isoformat(value)
+ elif value_type is time:
+ return 32
+ elif value_type is timedelta:
+ text = _trusted_timedelta_text(value)
+ elif value_type is UUID:
+ text = UUID.__str__(value)
+ else:
+ raise AssertionError(f"unaccounted normalized scalar: {value_type!r}")
+ size = _json_string_size(text, MAX_QUERY_RESULT_STRING_BYTES)
+ assert size is not None
+ return size
+
+
+def _pydantic_scalar_json_size(value: Any) -> int:
+ """Return the exact Pydantic wire size for a normalized scalar.
+
+ Source-result accounting deliberately retains its existing conservative
+ scalar rules. Final response projections, however, must match
+ pydantic-core's JSON number spelling: for example, it emits ``0.00001`` for
+ ``1e-5`` and ``1e-6`` for ``1e-6`` rather than Python's repr spellings.
"""
- if not isinstance(result, Mapping):
+ if type(value) is float:
+ return len(to_json(value))
+ return _normalized_scalar_json_size(value)
+
+
+def _charge_json_bytes(
+ budget: _ResultBudget, size: int, *, metadata: bool = False
+) -> str | None:
+ """Charge aggregate response bytes and the independent metadata
allowance."""
+ budget.json_bytes += size
+ if budget.json_bytes > MAX_QUERY_RESULT_VALUE_BYTES:
+ return "exceeds the total JSON-encoded byte limit"
+ if metadata:
+ budget.metadata_bytes += size
+ if budget.metadata_bytes > MAX_QUERY_RESULT_METADATA_BYTES:
+ return "metadata exceeds the total JSON-encoded byte limit"
+ return None
+
+
+def _integer_failure(value: int) -> str | None:
+ """Validate exact integer magnitude before decimal rendering or hashing."""
+ bits = int.bit_length(value)
+ if bits > MAX_QUERY_RESULT_INTEGER_BITS:
+ return "contains an integer exceeding the bit-length limit"
+ digits = 1 if bits == 0 else ((bits - 1) * 30103) // 100000 + 1
+ if digits > MAX_QUERY_RESULT_INTEGER_DIGITS:
+ return "contains an integer exceeding the digit limit"
+ return None
+
+
+def _decimal_failure(value: Decimal) -> str | None:
+ """Validate exact Decimal storage, finiteness, digits, and exponent."""
+ if Decimal.__sizeof__(value) > MAX_QUERY_RESULT_DECIMAL_STORAGE:
+ return "contains a Decimal exceeding the storage limit"
+ if not Decimal.is_finite(value):
+ return "contains a non-finite Decimal"
+ parts = Decimal.as_tuple(value)
+ if tuple.__len__(parts.digits) > MAX_QUERY_RESULT_DECIMAL_DIGITS:
+ return "contains a Decimal exceeding the digit limit"
+ exponent = parts.exponent
+ if type(exponent) is not int or abs(exponent) >
MAX_QUERY_RESULT_DECIMAL_EXPONENT:
+ return "contains a Decimal exceeding the exponent limit"
+ return None
+
+
+def _exact_object_namespace(value: Any) -> dict[str, Any] | None:
+ """Read an object's concrete storage without descriptor dispatch."""
+ try:
+ namespace = object.__getattribute__(value, "__dict__")
+ except (AttributeError, TypeError):
+ return None
+ return namespace if type(namespace) is dict else None
+
+
+def _dateutil_timezone_name_without_hooks(tzinfo: Any) -> str | None:
+ """Read a dateutil tzfile's IANA name from exact internal storage."""
+ tzinfo_type = type(tzinfo)
+ if tzinfo_type not in {_DATEUTIL_TZFILE_TYPE, dateutil_zoneinfo_tzfile}:
+ return None
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None:
+ return None
+ filename = dict.get(namespace, "_filename")
+ if type(filename) is not str or _bounded_utf8_length(filename, 4096) is
None:
+ return None
+ if tzinfo_type is dateutil_zoneinfo_tzfile:
+ name = filename
+ else:
+ marker = "/zoneinfo/"
+ marker_offset = str.find(filename, marker)
+ if marker_offset >= 0:
+ name = str.__getitem__(filename, slice(marker_offset +
len(marker), None))
+ elif not str.startswith(filename, "/") and str.find(filename, "\\") <
0:
+ name = filename
+ else:
+ return None
+ parts = str.split(name, "/")
+ if not parts or any(part in {"", ".", ".."} for part in parts):
+ return None
+ return name if _bounded_utf8_length(name, 256) is not None else None
+
+
+def _dateutil_ttinfo_without_hooks(
+ value: Any,
+) -> tuple[int, timedelta] | None:
+ """Read one exact dateutil transition record without user-hook dispatch."""
+ if type(value) is not dateutil_ttinfo:
+ return None
+ try:
+ offset = object.__getattribute__(value, "offset")
+ delta = object.__getattribute__(value, "delta")
+ except (AttributeError, TypeError):
+ return None
+ if type(offset) is not int or type(delta) is not timedelta:
+ return None
+ try:
+ if delta != timedelta(seconds=offset):
+ return None
+ except OverflowError:
+ return None
+ return offset, delta
+
+
+def _dateutil_named_offset_without_hooks( # noqa: C901
+ value: datetime, tzinfo: Any
+) -> timezone | None:
+ """Recover the offset selected by an exact dateutil named timezone.
+
+ A dateutil tzfile's finite transition table is its wire-semantic source of
+ truth. Reinterpreting its wall time through a system ``ZoneInfo`` database
+ changes negative-DST folds, nonexistent times, and dates after the final
+ transition. This mirrors dateutil's transition selection using only exact
+ builtin containers and its exact trusted transition-record type.
+ """
+ if _dateutil_timezone_name_without_hooks(tzinfo) is None:
+ return None
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None:
+ return None
+ transitions = dict.get(namespace, "_trans_list")
+ transition_info = dict.get(namespace, "_trans_idx")
+ standard_info = dict.get(namespace, "_ttinfo_std")
+ before_info = dict.get(namespace, "_ttinfo_before")
+ transition_count = tuple.__len__(transitions) if type(transitions) is
tuple else 0
+ if (
+ type(transitions) is not tuple
+ or type(transition_info) is not tuple
+ or tuple.__len__(transitions) != tuple.__len__(transition_info)
+ or transition_count > _MAX_DATEUTIL_TRANSITIONS
+ or _dateutil_ttinfo_without_hooks(standard_info) is None
+ or (
+ transition_count > 0 and
_dateutil_ttinfo_without_hooks(before_info) is None
+ )
+ ):
+ return None
+
+ previous: int | None = None
+ for transition in transitions:
+ if (
+ type(transition) is not int
+ or int.bit_length(transition) > 63
+ or (previous is not None and transition < previous)
+ ):
+ return None
+ previous = transition
+ if any(_dateutil_ttinfo_without_hooks(info) is None for info in
transition_info):
+ return None
+
+ naive = datetime(
+ value.year,
+ value.month,
+ value.day,
+ value.hour,
+ value.minute,
+ value.second,
+ value.microsecond,
+ )
+ try:
+ timestamp = (naive - EPOCH).total_seconds()
+ except (OverflowError, TypeError, ValueError):
+ return None
+
+ index: int | None = (
+ bisect_right(transitions, timestamp) - 1 if transition_count else None
+ )
+
+ def info_at(selected: int | None) -> Any:
+ if selected is None or selected + 1 >= transition_count:
+ return standard_info
+ if selected < 0:
+ return before_info
+ return tuple.__getitem__(transition_info, selected)
+
+ if index is not None and index != 0:
+ current = _dateutil_ttinfo_without_hooks(info_at(index))
+ prior = _dateutil_ttinfo_without_hooks(info_at(index - 1))
+ if current is None or prior is None:
+ return None
+ offset_delta = prior[0] - current[0]
+ transition = tuple.__getitem__(transitions, index)
+ is_ambiguous = timestamp < transition + offset_delta
+ index -= int(not value.fold and is_ambiguous)
+
+ selected = _dateutil_ttinfo_without_hooks(info_at(index))
+ if selected is None:
+ return None
+ try:
+ return timezone(selected[1])
+ except ValueError:
+ return None
+
+
+def _pytz_timezone_name_without_hooks(tzinfo: Any) -> str | None:
+ """Read and verify one generated pytz named-zone implementation."""
+ value_type = type(tzinfo)
+ if not _mro_contains(_type_mro(value_type), _PYTZ_NAMED_BASE_TYPES):
+ return None
+ try:
+ namespace = type.__getattribute__(value_type, "__dict__")
+ except (AttributeError, TypeError):
+ return None
+ if type(namespace) is not MappingProxyType:
+ return None
+ zone = namespace.get("zone")
+ if type(zone) is not str or _bounded_utf8_length(zone, 256) is None:
+ return None
+ try:
+ canonical = pytz.timezone(zone)
+ except (KeyError, ValueError):
+ return None
+ # A user subclass can inherit pytz's base and spoof ``zone``. Only the
+ # concrete class generated and cached by pytz for that name is trusted.
+ return zone if type(canonical) is value_type else None
+
+
+def _fixed_offset_without_hooks(tzinfo: Any) -> timezone | None:
+ """Reconstruct trusted dateutil/pytz fixed offsets from exact storage."""
+ if type(tzinfo) not in {_DATEUTIL_TZOFFSET_TYPE, _PYTZ_FIXED_OFFSET_TYPE}:
+ return None
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None:
+ return None
+ offset = dict.get(namespace, "_offset")
+ if type(offset) is not timedelta:
+ return None
+ try:
+ return timezone(offset)
+ except ValueError:
+ return None
+
+
+def _pytz_named_offset_without_hooks(tzinfo: Any) -> timezone | None:
+ """Return a localized pytz instance's stored offset without its hooks."""
+ if _pytz_timezone_name_without_hooks(tzinfo) is None:
+ return None
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None:
+ return None
+ offset = dict.get(namespace, "_utcoffset")
+ if type(offset) is not timedelta:
+ return None
+ try:
+ return timezone(offset)
+ except ValueError:
+ return None
+
+
+def _dateutil_local_offset_without_hooks(
+ value: datetime, tzinfo: Any
+) -> timezone | None:
+ """Select an exact dateutil-local offset using builtin system time data."""
+ if type(tzinfo) is not _DATEUTIL_TZLOCAL_TYPE:
+ return None
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None:
+ return None
+ standard_offset = dict.get(namespace, "_std_offset")
+ daylight_offset = dict.get(namespace, "_dst_offset")
+ has_daylight = dict.get(namespace, "_hasdst")
+ if (
+ type(standard_offset) is not timedelta
+ or type(daylight_offset) is not timedelta
+ or type(has_daylight) is not bool
+ ):
+ return None
+ selected_offset = standard_offset
+ if has_daylight:
+ epoch = datetime(1970, 1, 1)
+ naive = datetime(
+ value.year,
+ value.month,
+ value.day,
+ value.hour,
+ value.minute,
+ value.second,
+ value.microsecond,
+ )
+ timestamp = (naive - epoch).total_seconds()
+ try:
+ is_daylight = bool(
+ system_time.localtime(timestamp +
system_time.timezone).tm_isdst
+ )
+ daylight_saved = daylight_offset - standard_offset
+ previous_is_daylight = bool(
+ system_time.localtime(
+ timestamp
+ - timedelta.total_seconds(daylight_saved)
+ + system_time.timezone
+ ).tm_isdst
+ )
+ except (OverflowError, OSError, ValueError):
+ return None
+ is_ambiguous = not is_daylight and is_daylight != previous_is_daylight
+ if is_ambiguous:
+ is_daylight = not bool(value.fold)
+ selected_offset = daylight_offset if is_daylight else standard_offset
+ try:
+ return timezone(selected_offset)
+ except ValueError:
return None
+
+def _canonical_timezone(tzinfo: Any) -> timezone | ZoneInfo | None:
+ """Return an exact trusted timezone without invoking the source's
methods."""
+ if any(type(tzinfo) is type_ for type_ in _TRUSTED_TZINFO_TYPES):
+ return tzinfo
+ if type(tzinfo) in {_DATEUTIL_TZUTC_TYPE, _PYTZ_UTC_TYPE}:
+ return timezone.utc
+ if fixed_offset := _fixed_offset_without_hooks(tzinfo):
+ return fixed_offset
+ zone_name = _dateutil_timezone_name_without_hooks(
+ tzinfo
+ ) or _pytz_timezone_name_without_hooks(tzinfo)
+ if zone_name:
+ try:
+ return ZoneInfo(zone_name)
+ except (KeyError, ValueError, ZoneInfoNotFoundError):
+ return None
+ return None
+
+
+def _timestamp_offset_without_hooks(value: pd.Timestamp) -> timezone | None:
+ """Recover a timestamp's stored wall-clock offset without timezone
hooks."""
+ unit_multipliers = {"s": 1_000_000_000, "ms": 1_000_000, "us": 1_000,
"ns": 1}
+ multiplier = unit_multipliers.get(value.unit)
+ if multiplier is None:
+ return None
+ try:
+ instant_ns = int(value.asm8.view("i8")) * multiplier
+ epoch_ordinal = date.toordinal(date(1970, 1, 1))
+ wall_ns = (
+ (
+ (datetime.toordinal(value) - epoch_ordinal) * 86_400
+ + value.hour * 3600
+ + value.minute * 60
+ + value.second
+ )
+ * 1_000_000_000
+ + value.microsecond * 1000
+ + value.nanosecond
+ )
+ offset_ns = wall_ns - instant_ns
+ if offset_ns % 1000:
+ return None
+ return timezone(timedelta(microseconds=offset_ns // 1000))
+ except (OverflowError, TypeError, ValueError):
+ return None
+
+
+def _trusted_datetime_value(
+ value: datetime,
+) -> tuple[datetime | None, str | None]:
+ """Return an exact datetime rebuilt with only trusted timezone types."""
+ tzinfo = value.tzinfo
+ canonical_value = value
+ if tzinfo is not None and not any(
+ type(tzinfo) is trusted for trusted in _TRUSTED_TZINFO_TYPES
+ ):
+ canonical_tz: timezone | ZoneInfo | None
+ if _dateutil_timezone_name_without_hooks(tzinfo) is not None:
+ # A recognized dateutil tzfile must use its own finite transition
+ # table. Falling through to ZoneInfo would silently reinterpret a
+ # source-selected gap/fold or post-table wall time.
+ canonical_tz = _dateutil_named_offset_without_hooks(value, tzinfo)
+ else:
+ canonical_tz = (
+ _pytz_named_offset_without_hooks(tzinfo)
+ or _dateutil_local_offset_without_hooks(value, tzinfo)
+ or _canonical_timezone(tzinfo)
+ )
+ if canonical_tz is None:
+ return None, "contains a datetime with an unsupported timezone"
+ canonical_value = datetime(
+ value.year,
+ value.month,
+ value.day,
+ value.hour,
+ value.minute,
+ value.second,
+ value.microsecond,
+ tzinfo=canonical_tz,
+ fold=value.fold,
+ )
+ try:
+ # Exercise builtin validation without dispatching through a source
+ # timezone after the reconstruction above.
+ datetime.isoformat(canonical_value)
+ except (OverflowError, TypeError, ValueError):
+ return None, "contains an invalid datetime"
+ return canonical_value, None
+
+
+def _trusted_datetime_text(value: datetime) -> tuple[str | None, str | None]:
+ """Serialize an exact Python datetime through only trusted timezone
types."""
+ canonical_value, reason = _trusted_datetime_value(value)
+ if reason is not None or canonical_value is None:
+ return None, reason or "contains an invalid datetime"
+ return datetime.isoformat(canonical_value), None
+
+
+def _trusted_time_text(value: time) -> tuple[str | None, str | None]:
+ """Serialize an exact Python time through only trusted timezone types."""
+ tzinfo = value.tzinfo
+ canonical_value = value
+ if tzinfo is not None and not any(
+ type(tzinfo) is trusted for trusted in _TRUSTED_TZINFO_TYPES
+ ):
+ canonical_tz = _canonical_timezone(tzinfo)
+ if canonical_tz is None and type(tzinfo) is _DATEUTIL_TZLOCAL_TYPE:
+ namespace = _exact_object_namespace(tzinfo)
+ if namespace is None or type(dict.get(namespace, "_hasdst")) is
not bool:
+ return None, "contains a time with an unsupported timezone"
+ if dict.get(namespace, "_hasdst"):
+ canonical_tz = None
+ else:
+ standard_offset = dict.get(namespace, "_std_offset")
+ if type(standard_offset) is not timedelta:
+ return None, "contains a time with an unsupported timezone"
+ try:
+ canonical_tz = timezone(standard_offset)
+ except ValueError:
+ return None, "contains a time with an unsupported timezone"
+ elif canonical_tz is None:
+ return None, "contains a time with an unsupported timezone"
+ canonical_value = time(
+ value.hour,
+ value.minute,
+ value.second,
+ value.microsecond,
+ tzinfo=canonical_tz,
+ fold=value.fold,
+ )
+ try:
+ return time.isoformat(canonical_value), None
+ except (OverflowError, TypeError, ValueError):
+ return None, "contains an invalid time"
+
+
+def _trusted_timestamp_value(
+ value: pd.Timestamp,
+) -> tuple[pd.Timestamp | None, str | None]:
+ """Return a timestamp rebuilt with only trusted timezone
implementations."""
+ tzinfo = value.tzinfo
+ try:
+ if tzinfo is not None and not any(
+ type(tzinfo) is trusted for trusted in _TRUSTED_TZINFO_TYPES
+ ):
+ if (
+ _canonical_timezone(tzinfo) is None
+ and type(tzinfo) is not _DATEUTIL_TZLOCAL_TYPE
+ ):
+ return (
+ None,
+ "contains a pandas timestamp with an unsupported timezone",
+ )
+ if (canonical_tz := _timestamp_offset_without_hooks(value)) is
None:
+ return None, "contains an invalid pandas timestamp"
+ # Rebuild from the stored instant and resolution. No method on the
+ # original pytz/dateutil object is called, and the recovered fixed
+ # offset preserves the timestamp's selected fold.
+ raw_value = value.asm8.view("i8")
+ value = pd.Timestamp(raw_value, unit=value.unit,
tz="UTC").tz_convert(
+ canonical_tz
+ )
+ # Validate the retained resolution and selected UTC offset.
+ pd.Timestamp.isoformat(value)
+ except (KeyError, OverflowError, TypeError, ValueError):
+ return None, "contains an invalid pandas timestamp"
+ return value, None
+
+
+def _trusted_timestamp_text(value: pd.Timestamp) -> tuple[str | None, str |
None]:
+ """Convert an exact pandas timestamp to its canonical JSON
representation."""
+ canonical_value, reason = _trusted_timestamp_value(value)
+ if reason is not None or canonical_value is None:
+ return None, reason or "contains an invalid pandas timestamp"
+ # ISO output preserves nanoseconds and the UTC offset selected by fold.
+ text = pd.Timestamp.isoformat(canonical_value)
+ if _bounded_utf8_length(text, MAX_QUERY_RESULT_STRING_BYTES) is None:
+ return None, "contains an oversized pandas timestamp"
+ return text, None
+
+
+def _normalize_trusted_scalar( # noqa: C901
+ value: Any, *, max_string_bytes: int = MAX_QUERY_RESULT_STRING_BYTES
+) -> tuple[Any, str | None]:
+ """Normalize one exact trusted pandas/NumPy scalar or validate a builtin.
+
+ Type identity is checked before every conversion. This deliberately does
not
+ accept subclasses or generic ``np.generic``/pandas extension objects, whose
+ conversion hooks are outside the trusted ChartData materialization
contract.
+ """
+ value_type = type(value)
+ enum_seen: set[int] = set()
+ while _mro_contains(_type_mro(value_type), (Enum,)):
+ identity = id(value)
+ if identity in enum_seen or len(enum_seen) >= _MAX_ROW_CONTAINER_DEPTH:
+ return None, "contains a recursive enum"
+ enum_seen.add(identity)
+ try:
+ value = object.__getattribute__(value, "_value_")
+ except Exception:
+ return None, "contains an unsupported enum"
+ value_type = type(value)
+
+ if value is None or value_type is bool:
+ return value, None
+ if value_type is str:
+ size = _bounded_utf8_length(value, max_string_bytes)
+ return (
+ (value, None)
+ if size is not None
+ else (
+ None,
+ "contains an invalid or oversized string",
+ )
+ )
+ if value_type is int:
+ return value, _integer_failure(value)
+ if value_type is float:
+ if math.isnan(value):
+ return None, None
+ if math.isinf(value):
+ return None, "contains a non-finite number"
+ return value, None
+ if value_type is Decimal:
+ return value, _decimal_failure(value)
+
+ if value_type is datetime:
+ return _trusted_datetime_text(value)
+ if value_type is time:
+ return _trusted_time_text(value)
+ if value_type is date:
+ return date.isoformat(value), None
+ if value_type is timedelta:
+ return _trusted_timedelta_text(value), None
+ if value_type is UUID:
+ return UUID.__str__(value), None
+
+ if value_type is _PANDAS_NAT_TYPE or value_type is _PANDAS_NA_TYPE:
+ return None, None
+ if value_type is pd.Timestamp:
+ return _trusted_timestamp_text(value)
+ if value_type is pd.Timedelta:
+ if pd.isna(value):
+ return None, None
+ text = pd.Timedelta.isoformat(value)
+ if _bounded_utf8_length(text, MAX_QUERY_RESULT_STRING_BYTES) is None:
+ return None, "contains an oversized pandas timedelta"
+ return text, None
+ if value_type is _PANDAS_PERIOD_TYPE or value_type is
_PANDAS_INTERVAL_TYPE:
+ # The concrete extension scalar implementations are trusted, unlike an
+ # arbitrary subclass's ``__str__`` implementation.
+ text = str(value)
+ if _bounded_utf8_length(text, MAX_QUERY_RESULT_STRING_BYTES) is None:
+ return None, "contains an oversized pandas scalar"
+ return text, None
+
+ if any(value_type is type_ for type_ in _NUMPY_INTEGER_TYPES):
+ normalized_integer = int(value)
+ return normalized_integer, _integer_failure(normalized_integer)
+ if any(value_type is type_ for type_ in _NUMPY_FLOAT_TYPES):
+ normalized_float = float(value)
Review Comment:
A trusted `np.longdouble("1.000000000000000001")` survives DataFrame
materialization unchanged but becomes `1.0` here before chart, dataset, and
table responses or exports, silently losing precision outside the renderer.
Could extended-precision values keep an exact wire representation instead of
narrowing to binary64?
##########
superset/mcp_service/chart/compile.py:
##########
@@ -282,6 +329,427 @@ 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":
+ 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":
+ matches = [
+ item
+ for item in saved_metrics
+ if item == name or item.casefold() == name.casefold()
+ ]
+ if len(set(matches)) != 1:
Review Comment:
The exact-match fix handles metric roles, but the HAVING branch still
gathers all case-folded matches: an exact `HAVING Revenue` falls through to
physical-column validation when saved metrics include both `Revenue` and
`revenue`, so the valid chart update is still rejected. Could that branch
prefer exact saved-metric names too?
--
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]
