Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/admin_docs/configuration/mcp-server.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,37 @@ supported time grains; missing or empty lists use the dataset's supported grains
Pass `[]` to clear an optional time-grain filter; required filters cannot be
cleared.

## Reselecting fields on an existing semantic chart

`update_chart` can certify complete field reselection for table and interactive
table charts whose semantic selection marker is missing or outdated. Supply all
columns using the target view's member IDs and explicitly supply `filters`
(`[]` clears them). Supply sorting explicitly if the chart retains sorting.
Certification applies only when the final merged query contains no unverified
retained selections; sending `config` alone is not enough.

Only an explicit `filters: []` authorizes clearing saved WHERE/HAVING predicate
aliases, including legacy and adhoc filters. A non-empty filter replacement that
would discard those unclassified predicates is refused; reselect in Explore, or
explicitly clear filters if removing the saved constraints is intended.

If native controls, temporal bindings, external filters, secondary queries or
unsupported plugin state cannot be verified, the update refuses without changing
the saved chart. The error names the incomplete category and directs you to
reselect it in Explore and save there. Omitted state is not silently reset.
For example: “Cannot verify temporal selections; reselect them in Explore and
save the chart.”

Renaming a chart or using `add_columns` does not upgrade its selection marker.
Ordinary partial updates on the same source with a current marker keep their
existing behavior. Changing the source requires fresh target selections even if
the two sources use the same marker string. On versioned providers, a source-only
rebind without a complete table config is refused as “no reselection supplied”;
retry with complete selections or reselect in Explore. A preview is a snapshot, not a lease:
a concurrent saved-chart change can cause a direct update to refuse and require
a reload. Providers without a selection-version contract and physical datasets
do not receive an invented marker.

## Missing chart columns

`generate_chart`, `update_chart`, `update_chart_preview` and
Expand Down
78 changes: 77 additions & 1 deletion superset/commands/chart/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@
# specific language governing permissions and limitations
# under the License.
import logging
from dataclasses import asdict, dataclass
from datetime import datetime
from functools import partial
from typing import Any, Optional, TYPE_CHECKING
from typing import Any, cast, Optional, TYPE_CHECKING

from flask import g
from flask_appbuilder.models.sqla import Model
from marshmallow import ValidationError
from sqlalchemy import Table
from sqlalchemy.engine import CursorResult
from sqlalchemy.orm import lazyload

from superset import security_manager
from superset.commands.base import BaseCommand, UpdateMixin
Expand Down Expand Up @@ -72,20 +76,47 @@ def is_query_context_update(properties: dict[str, Any]) -> bool:
)


@dataclass(frozen=True)
class ChartUpdateBasis:
"""Capture the saved inputs used to prepare a certified chart update."""

params: str | None
datasource_id: int | None
datasource_type: str | None
viz_type: str | None
slice_name: str | None

@classmethod
def from_chart(cls, chart: Slice) -> "ChartUpdateBasis":
"""Snapshot values rather than retaining a mutable ORM reference."""
return cls(
chart.params,
chart.datasource_id,
chart.datasource_type,
chart.viz_type,
chart.slice_name,
)


class UpdateChartCommand(UpdateMixin, BaseCommand):
def __init__(
self,
model_id: int,
data: dict[str, Any],
normalization_changes: object = None,
*,
expected_basis: ChartUpdateBasis | None = None,
) -> None:
self._model_id: int = model_id
self._properties: dict[str, Any] = data.copy()
self._model: Optional[Slice] = None
self._normalization_changes: object = normalization_changes
self._expected_basis: ChartUpdateBasis | None = expected_basis

@transaction(on_error=partial(on_error, reraise=ChartUpdateFailedError))
def run(self) -> Model:
if self._expected_basis is not None:
self._fence_expected_basis()
self.validate()
assert self._model

Expand Down Expand Up @@ -126,6 +157,51 @@ def run(self) -> Model:

return ChartDAO.update(self._model, self._properties)

def _fence_expected_basis(self) -> None:
"""Reject stale preparation and hold its write fence through commit."""
assert self._expected_basis is not None
table: Table = Slice.__table__
# Core UPDATE avoids ORM history for the guard. Pin onupdate columns to
# themselves so acquiring the fence does not change audit timestamps.
values: dict[Any, Any] = {
column: column for column in table.c if column.onupdate is not None
}
values[table.c.id] = table.c.id
with db.session.no_autoflush:
result: CursorResult = cast(
CursorResult,
db.session.execute(
table.update()
.where(
table.c.id == self._model_id,
*(
table.c[key].is_not_distinct_from(value)
for key, value in asdict(self._expected_basis).items()
),
)
.values(values)
),
)
if result.rowcount != 1:
raise ChartUpdateFailedError("Chart changed; reload before retrying.")
# The locking read also escapes MySQL REPEATABLE READ snapshots.
# Keep a strong reference until the command's normal ORM mutation.
self._model = (
db.session.query(Slice)
# Lock only the chart: subquery eager loads can propagate FOR
# UPDATE into DISTINCT queries that PostgreSQL rejects.
.options(lazyload("*"))
.populate_existing()
.filter(Slice.id == self._model_id)
.with_for_update()
.one_or_none()
)
if (
self._model is None
or ChartUpdateBasis.from_chart(self._model) != self._expected_basis
):
raise ChartUpdateFailedError("Chart changed; reload before retrying.")

def _validate_new_dashboard_access(
self, requested_dashboards: list[Dashboard], exceptions: list[ValidationError]
) -> None:
Expand Down
89 changes: 76 additions & 13 deletions superset/mcp_service/chart/chart_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@

from superset.common.utils.time_grain_utils import apply_time_grain_to_base_axis
from superset.constants import EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS
from superset.mcp_service.dashboard_scope import (
merge_semantic_selection_sources,
SEMANTIC_SELECTION_MEMBER_KEYS,
)
from superset.utils.core import ExtraFiltersReasonType

if TYPE_CHECKING:
Expand Down Expand Up @@ -320,6 +324,7 @@ def merge_extra_form_data(
merge, which iterates them, never receives ``None``.
"""
merged: dict[str, Any] = dict(existing) if isinstance(existing, dict) else {}
sources: list[Any] = merge_semantic_selection_sources(merged, incoming)
for key in ("adhoc_filters", "filters"):
if key in merged and merged[key] is None:
del merged[key]
Expand All @@ -333,6 +338,8 @@ def merge_extra_form_data(
merged[key] = {**current, **value}
else:
merged[key] = value
if sources:
merged["semantic_selection_sources"] = sources
return merged


Expand Down Expand Up @@ -427,6 +434,19 @@ def merge_extra_form_data_filters_into_query(
datasource_type: str,
) -> None:
"""Merge request extra_form_data predicates into an existing query payload."""
selection_version: str | None = (
_query_semantic_selection_version(
{
"datasource": f"{datasource_id}__{datasource_type}",
"semantic_selection_version": (query.get("extras") or {}).get(
"semantic_selection_version"
),
},
extra_form_data,
)
if datasource_type == "semantic_view"
else None
)
extra_query_form_data: dict[str, Any] = {"adhoc_filters": []}
prepare_form_data_for_query(
extra_query_form_data,
Expand All @@ -435,6 +455,8 @@ def merge_extra_form_data_filters_into_query(
extra_form_data,
)
merge_form_data_filters_into_query(query, extra_query_form_data)
if selection_version is not None:
query.setdefault("extras", {})["semantic_selection_version"] = selection_version
if extra_form_data.get("time_range"):
from superset.charts.data.dashboard_filter_context import (
mark_dashboard_time_range_filters,
Expand Down Expand Up @@ -1165,6 +1187,36 @@ def coerce_enum_values(value: Any) -> Any:
return value


def _query_semantic_selection_version(
form_data: dict[str, Any], extra_form_data: dict[str, Any] | None
) -> str | None:
"""Mirror Explore's version transport without certifying external selections."""
version: object = form_data.get("semantic_selection_version")
if not isinstance(version, str):
return None
unverified: str = "unverified-external-selections"
sources: list[Any] = list(form_data.get("semantic_selection_sources") or [])
extra: dict[str, Any]
for extra in (form_data.get("extra_form_data") or {}, extra_form_data or {}):
if extra.get("semantic_selection_sources"):
sources.extend(extra["semantic_selection_sources"])
elif any(extra.get(key) for key in SEMANTIC_SELECTION_MEMBER_KEYS):
return unverified
if any(
not isinstance(source, dict)
or source.get("datasource") != form_data.get("datasource")
or source.get("version") != version
for source in sources
):
return unverified
if any(
filter_.get("col") not in {"__time_range", "__time_grain", "__time_compare"}
for filter_ in form_data.get("extra_filters") or []
):
return unverified
return version


def build_query_dicts_from_form_data(
form_data: dict[str, Any],
datasource_id: Any,
Expand Down Expand Up @@ -1193,6 +1245,11 @@ def build_query_dicts_from_form_data(
form_data = coerce_enum_values(deepcopy(form_data))
if extra_form_data is not None:
extra_form_data = coerce_enum_values(deepcopy(extra_form_data))
selection_version: str | None = (
_query_semantic_selection_version(form_data, extra_form_data)
if datasource_type == "semantic_view"
else None
)
viz_type: str = (
form_data.get("viz_type")
or (getattr(chart, "viz_type", "") if chart else "")
Expand Down Expand Up @@ -1222,33 +1279,39 @@ def build_query_dicts_from_form_data(

# Plugins see (and may validate) the complete reconstructed state, including
# legacy clauses and request-level extra_form_data merged above.
queries: list[dict[str, Any]] | None = None
if plugin is not None:
plugin_queries = plugin.build_query_dicts(
queries = plugin.build_query_dicts(
form_data,
viz_type=viz_type,
engine=engine,
row_limit=row_limit,
order_desc=order_desc,
)
if plugin_queries is not None:
return plugin_queries

# Deck.gl charts use spatial column configs rather than the standard
# metrics / groupby fields. Extract columns from the spatial controls.
if viz_type.startswith("deck_"):
return [
if queries is None and viz_type.startswith("deck_"):
queries = [
build_deck_gl_query_dict(
form_data, viz_type, row_limit=row_limit, order_desc=order_desc
)
]
return build_query_objects_from_form_data(
form_data,
viz_type=viz_type,
row_limit=row_limit,
order_desc=order_desc,
filters_prepared=True,
secondary_form_data=secondary_form_data,
)
if queries is None:
queries = build_query_objects_from_form_data(
form_data,
viz_type=viz_type,
row_limit=row_limit,
order_desc=order_desc,
filters_prepared=True,
secondary_form_data=secondary_form_data,
)
if selection_version is not None:
for query in queries:
query.setdefault("extras", {})["semantic_selection_version"] = (
selection_version
)
return queries


def build_deck_gl_query_dict(
Expand Down
10 changes: 8 additions & 2 deletions superset/mcp_service/chart/chart_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,10 @@ def _add_xy_limits(form_data: Dict[str, Any], config: XYChartConfig) -> None:


def map_xy_config( # noqa: C901
config: XYChartConfig, dataset_id: int | str | None = None
config: XYChartConfig,
dataset_id: int | str | None = None,
*,
temporal_columns: set[str] | None = None,
) -> Dict[str, Any]:
"""Map XY chart config to form_data with defensive validation."""
# Early validation to prevent empty charts
Expand All @@ -1783,7 +1786,10 @@ def map_xy_config( # noqa: C901
# Check if x-axis column is truly temporal (based on actual SQL type).
# Reuse the dataset fetched above (if any) to avoid a second DAO lookup.
x_is_temporal = is_column_truly_temporal(
config.x.name, dataset_id, dataset=resolved_dataset
config.x.name,
dataset_id,
dataset=resolved_dataset,
temporal_columns=temporal_columns,
)

# Map chart kind to viz_type - always use the same viz types
Expand Down
9 changes: 9 additions & 0 deletions superset/mcp_service/chart/datasource_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ def resolve_semantic_view(view_id: int | str | UUID) -> ChartDatasource | None:
)


def stamp_new_semantic_selections(
form_data: dict[str, Any], target: ChartDatasource
) -> None:
"""Stamp freshly selected members, never retained selections from a saved chart."""
version: str | None = target.explorable.implementation.selection_identity_version
if isinstance(version, str):
form_data["semantic_selection_version"] = version


def build_context_from_explorable(target: ChartDatasource) -> DatasetContext:
"""Adapt authorized host metadata without assuming table columns."""
view: SemanticView = target.explorable
Expand Down
19 changes: 3 additions & 16 deletions superset/mcp_service/chart/plugins/xy.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,9 @@
from typing import Any, ClassVar

from superset.mcp_service.chart.chart_utils import (
_add_adhoc_filters,
_xy_chart_context,
_xy_chart_what,
configure_temporal_handling,
create_metric_object,
is_column_truly_temporal,
map_xy_config,
)
from superset.mcp_service.chart.plugin import BaseChartPlugin
Expand Down Expand Up @@ -299,19 +296,9 @@ def apply_temporal_columns(
form_data: dict[str, Any],
temporal_columns: set[str],
) -> None:
"""Undo dataset-less time defaults for a categorical semantic x-axis."""
if (
config.x is None
or config.x.name is None
or is_column_truly_temporal(
config.x.name, None, temporal_columns=temporal_columns
)
):
return
configure_temporal_handling(form_data, False, config.time_grain)
form_data.pop("adhoc_filters", None)
_add_adhoc_filters(form_data, config.filters)
form_data.pop("x_axis_time_format", None)
"""Rebuild with authoritative types before deciding sorting and warnings."""
form_data.clear()
form_data.update(map_xy_config(config, temporal_columns=temporal_columns))

def normalize_column_refs(self, config: Any, dataset_context: Any) -> Any:
config_dict = config.model_dump(exclude_unset=True)
Expand Down
Loading
Loading