You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/27 20:57:03 UTC
(superset) branch master updated: feat(mcp): add create_dataset_metric and delete_dataset_metric tools (#44580)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 25e63b54fd8 feat(mcp): add create_dataset_metric and delete_dataset_metric tools (#44580)
25e63b54fd8 is described below
commit 25e63b54fd86ebeaa5de1ff1f25342fcbdb1dfb4
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Sun Sep 27 16:56:57 2026 -0400
feat(mcp): add create_dataset_metric and delete_dataset_metric tools (#44580)
---
.../docs/using-superset/using-ai-with-superset.mdx | 19 +
superset/commands/dataset/update.py | 19 +-
superset/daos/dataset.py | 28 +-
superset/mcp_service/app.py | 7 +-
superset/mcp_service/dataset/schemas.py | 81 +++-
superset/mcp_service/dataset/tool/__init__.py | 4 +
.../dataset/tool/create_dataset_metric.py | 172 +++++++
.../dataset/tool/delete_dataset_metric.py | 246 ++++++++++
superset/mcp_service/mcp_config.py | 2 +
superset/mcp_service/utils/response_size_utils.py | 2 +
.../dataset/tool/test_dataset_metric_mutations.py | 529 +++++++++++++++++++++
.../mcp_service/test_mcp_tool_registration.py | 5 +
.../mcp_service/test_rbac_tool_enforcement.py | 12 +
.../unit_tests/mcp_service/test_tool_inventory.py | 2 +
14 files changed, 1114 insertions(+), 14 deletions(-)
diff --git a/docs/docs/using-superset/using-ai-with-superset.mdx b/docs/docs/using-superset/using-ai-with-superset.mdx
index 06d4914baa1..4943b51a819 100644
--- a/docs/docs/using-superset/using-ai-with-superset.mdx
+++ b/docs/docs/using-superset/using-ai-with-superset.mdx
@@ -265,8 +265,27 @@ Ask your admin for the MCP server URL and any authentication tokens you need.
| `list_datasets` | List datasets with filtering and search |
| `get_dataset_info` | Get dataset metadata (columns, metrics, schema details) |
| `create_virtual_dataset` | Create a virtual dataset from a SQL query |
+| `create_dataset_metric` | Add a saved metric with a unique name and SQL expression; accepts the same optional properties as `update_dataset_metric` (requires dataset editorship) |
+| `delete_dataset_metric` | Delete only the selected saved metric by ID, UUID, or name, preserving concurrently added metrics, and report accessible charts referencing it; chart definitions are not modified (requires dataset editorship) |
| `update_dataset_metric` | Update a saved metric's expression, name, verbose_name, or format (affects every chart using it; requires dataset ownership) |
+Saved-metric creation and deletion accept a dataset ID or UUID and require the
+same dataset write permission and editorship as metric updates. For example,
+call `create_dataset_metric` with
+`{"dataset_id": 123, "metric_name": "sum_revenue", "expression": "SUM(revenue)"}`.
+Optional properties include `verbose_name`, `description`, `d3format`, `currency`,
+`warning_text`, `metric_type`, and `extra`, just as for `update_dataset_metric`.
+Duplicate names are rejected; creation adds only the requested metric without
+replacing or rewriting existing metrics.
+
+Call `delete_dataset_metric` with
+`{"dataset_id": 123, "metric": "sum_revenue"}` to remove that metric. The response
+includes the deleted metric and `affected_charts`: accessible charts referencing
+its name in saved metric controls, HAVING filters, or query contexts. Warn users
+that those charts may need repair; deletion does not rewrite charts. References
+embedded in SQL expressions are not inferred. If a chart has malformed query-context
+JSON, impact reporting still checks its saved form data and other accessible charts.
+
### Charts
| Tool | Description |
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 38f2d3e704f..a23a10ca4ca 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -75,11 +75,16 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
model_id: int,
data: dict[str, Any],
override_columns: Optional[bool] = False,
+ *,
+ preserve_existing_metrics: bool = False,
+ delete_metric_ids: set[int] | None = None,
):
self._model_id = model_id
self._properties = data.copy()
self._model: Optional[SqlaTable] = None
self.override_columns = override_columns
+ self._delete_metric_ids = delete_metric_ids
+ self._preserve_existing_metrics = preserve_existing_metrics
self._properties["override_columns"] = override_columns
@transaction(
@@ -95,7 +100,12 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
def run(self) -> Model:
self.validate()
assert self._model
- return DatasetDAO.update(self._model, attributes=self._properties)
+ return DatasetDAO.update(
+ self._model,
+ attributes=self._properties,
+ preserve_existing_metrics=self._preserve_existing_metrics,
+ delete_metric_ids=self._delete_metric_ids,
+ )
def validate(self) -> None:
exceptions: list[ValidationError] = []
@@ -118,6 +128,13 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
self._validate_dataset_source(exceptions)
self._validate_semantics(exceptions)
+ if (
+ self._delete_metric_ids is not None
+ and not DatasetDAO.validate_metrics_exist(
+ self._model_id, list(self._delete_metric_ids)
+ )
+ ):
+ exceptions.append(DatasetMetricsNotFoundValidationError())
if exceptions:
raise DatasetInvalidError(exceptions=exceptions)
diff --git a/superset/daos/dataset.py b/superset/daos/dataset.py
index dfd72ad6080..36ad8f15270 100644
--- a/superset/daos/dataset.py
+++ b/superset/daos/dataset.py
@@ -423,11 +423,23 @@ class DatasetDAO(BaseDAO[SqlaTable]):
cls,
item: SqlaTable | None = None,
attributes: dict[str, Any] | None = None,
+ *,
+ preserve_existing_metrics: bool = False,
+ delete_metric_ids: set[int] | None = None,
) -> SqlaTable:
"""
- Updates a Dataset model on the metadata DB
+ Updates a Dataset model on the metadata DB.
+
+ delete_metric_ids removes only explicitly selected metrics, without
+ replacing or replaying the remaining metrics.
"""
+ if item and delete_metric_ids:
+ for metric in item.metrics:
+ if metric.id in delete_metric_ids:
+ db.session.delete(metric)
+ attributes = {**(attributes or {}), "changed_on": datetime.now()}
+
if item and attributes:
force_update: bool = False
if "columns" in attributes:
@@ -439,7 +451,11 @@ class DatasetDAO(BaseDAO[SqlaTable]):
force_update = True
if "metrics" in attributes:
- cls.update_metrics(item, attributes.pop("metrics"))
+ metrics = attributes.pop("metrics")
+ if preserve_existing_metrics:
+ cls.update_metrics(item, metrics, preserve_existing=True)
+ else:
+ cls.update_metrics(item, metrics)
force_update = True
if force_update:
@@ -584,6 +600,8 @@ class DatasetDAO(BaseDAO[SqlaTable]):
cls,
model: SqlaTable,
property_metrics: list[dict[str, Any]],
+ *,
+ preserve_existing: bool = False,
) -> None:
"""
Creates/updates and/or deletes a list of metrics, based on a
@@ -592,7 +610,7 @@ class DatasetDAO(BaseDAO[SqlaTable]):
- If a metric Dict has an `id` property then we update.
- If a metric Dict does not have an `id` then we create a new metric.
- If there are extra metrics on the metadata db that are not defined on the List
- then we delete.
+ then we delete, unless preserve_existing is set.
Uses individual ORM operations (not bulk) so that SQLAlchemy-Continuum
can capture each row change in the version history.
@@ -617,6 +635,10 @@ class DatasetDAO(BaseDAO[SqlaTable]):
for key, value in properties.items():
setattr(metric, key, value)
+ # Additive mutations must not delete metrics absent from their payload.
+ if preserve_existing:
+ return
+
# Delete removed metrics
ids_to_keep = property_metrics_by_id.keys()
for metric in model.metrics:
diff --git a/superset/mcp_service/app.py b/superset/mcp_service/app.py
index 236aec2b418..e8a285070ff 100644
--- a/superset/mcp_service/app.py
+++ b/superset/mcp_service/app.py
@@ -187,6 +187,8 @@ Dataset Management:
- get_dataset_info: Get detailed dataset information by ID (includes columns/metrics)
- create_dataset: Register a physical table as a dataset against an existing DB connection (requires write access)
- create_virtual_dataset: Save a SQL query as a virtual dataset for charting (requires write access)
+- create_dataset_metric: Add a saved metric to a dataset (requires dataset editorship)
+- delete_dataset_metric: Delete a saved metric and report referencing charts (requires dataset editorship)
- update_dataset_metric: Update a saved metric on a dataset — expression, name, verbose_name, format (requires dataset ownership)
- query_dataset: Query a dataset using its semantic layer (saved metrics, dimensions, filters) without needing a saved chart
@@ -517,7 +519,8 @@ Input format:
{_instance_info_role_bullet}- ALWAYS check the user's roles BEFORE suggesting write operations (creating datasets,
charts, or dashboards). SQL execution is a separate permission — see execute_sql below.
- Write tools (generate_chart, generate_dashboard, update_chart, update_dashboard,
- duplicate_dashboard, create_dataset, create_virtual_dataset, update_dataset_metric,
+ duplicate_dashboard, create_dataset, create_virtual_dataset, create_dataset_metric,
+ delete_dataset_metric, update_dataset_metric,
save_sql_query, add_chart_to_existing_dashboard, manage_native_filters,
remove_chart_from_dashboard, update_chart_preview, manage_dashboard_owners,
manage_dashboard_roles, manage_dashboard_certification) require write
@@ -838,7 +841,9 @@ from superset.mcp_service.database.tool import ( # noqa: F401, E402
)
from superset.mcp_service.dataset.tool import ( # noqa: F401, E402
create_dataset,
+ create_dataset_metric,
create_virtual_dataset,
+ delete_dataset_metric,
get_dataset_info,
list_datasets,
query_dataset,
diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py
index 3cc6b594dc4..9c7510c5136 100644
--- a/superset/mcp_service/dataset/schemas.py
+++ b/superset/mcp_service/dataset/schemas.py
@@ -627,8 +627,8 @@ class MetricCurrency(BaseModel):
)
-class UpdateDatasetMetricRequest(BaseModel):
- """Request schema for update_dataset_metric."""
+class DatasetMetricProperties(BaseModel):
+ """Dataset identifier and writable saved-metric properties."""
model_config = ConfigDict(populate_by_name=True)
@@ -637,12 +637,6 @@ class UpdateDatasetMetricRequest(BaseModel):
description="Dataset identifier — numeric ID or UUID string. "
"Use list_datasets to find valid IDs.",
)
- metric: int | str = Field(
- ...,
- description="Metric to update — numeric metric ID, metric UUID, or "
- "metric_name (e.g. 'sum_revenue'). Numeric strings are treated as IDs. "
- "Use get_dataset_info to discover a dataset's saved metrics.",
- )
metric_name: str | None = Field(
None,
max_length=255,
@@ -691,7 +685,7 @@ class UpdateDatasetMetricRequest(BaseModel):
)
@model_validator(mode="after")
- def validate_updates(self) -> "UpdateDatasetMetricRequest":
+ def validate_updates(self) -> "DatasetMetricProperties":
"""Require at least one updatable property and reject empty/invalid values.
Guards against no-op requests, empty ``metric_name``/``expression``, and
@@ -715,6 +709,39 @@ class UpdateDatasetMetricRequest(BaseModel):
return self
+class UpdateDatasetMetricRequest(DatasetMetricProperties):
+ """Request schema for update_dataset_metric."""
+
+ metric: int | str = Field(
+ ...,
+ description="Metric to update — numeric metric ID, metric UUID, or "
+ "metric_name (e.g. 'sum_revenue'). Numeric strings are treated as IDs. "
+ "Use get_dataset_info to discover a dataset's saved metrics.",
+ )
+
+
+class CreateDatasetMetricRequest(DatasetMetricProperties):
+ """Request schema for create_dataset_metric."""
+
+ metric_name: str = Field(
+ ..., max_length=255, description="Metric name, unique within the dataset."
+ )
+ expression: str = Field(
+ ..., description="SQL aggregation expression (e.g. 'SUM(revenue)')."
+ )
+
+
+class DeleteDatasetMetricRequest(BaseModel):
+ """Request schema for delete_dataset_metric."""
+
+ dataset_id: int | str = Field(
+ ..., description="Dataset identifier — numeric ID or UUID string."
+ )
+ metric: int | str = Field(
+ ..., description="Metric ID, UUID, or metric_name. Numeric strings are IDs."
+ )
+
+
class DatasetMetricDetail(SqlMetricInfo):
"""Full saved-metric details, including identifiers."""
@@ -750,6 +777,42 @@ class UpdateDatasetMetricResponse(BaseModel):
)
+class CreateDatasetMetricResponse(BaseModel):
+ """Response schema for create_dataset_metric."""
+
+ dataset_id: int | None = Field(None, description="Dataset ID")
+ dataset_name: str | None = Field(None, description="Dataset name")
+ metric: DatasetMetricDetail | None = Field(
+ None, description="Created metric, or null if creation failed."
+ )
+ url: str | None = Field(None, description="Explore URL for the dataset")
+ error: str | None = Field(None, description="Error message, or null on success")
+
+
+class MetricChartReference(BaseModel):
+ """An accessible chart referencing a saved metric by name."""
+
+ id: int = Field(..., description="Chart ID")
+ uuid: str | None = Field(None, description="Chart UUID")
+ slice_name: str = Field(..., description="Chart name")
+
+
+class DeleteDatasetMetricResponse(BaseModel):
+ """Response schema for delete_dataset_metric."""
+
+ dataset_id: int | None = Field(None, description="Dataset ID")
+ dataset_name: str | None = Field(None, description="Dataset name")
+ metric: DatasetMetricDetail | None = Field(
+ None, description="Deleted metric, or null if deletion failed."
+ )
+ affected_charts: list[MetricChartReference] = Field(
+ default_factory=list,
+ description="Accessible charts referencing the deleted metric by name. "
+ "Their definitions are not modified and may need repair.",
+ )
+ error: str | None = Field(None, description="Error message, or null on success")
+
+
VALID_FILTER_OPS = Literal[
"==",
"!=",
diff --git a/superset/mcp_service/dataset/tool/__init__.py b/superset/mcp_service/dataset/tool/__init__.py
index 191ffd4c4d8..0f96a549c9b 100644
--- a/superset/mcp_service/dataset/tool/__init__.py
+++ b/superset/mcp_service/dataset/tool/__init__.py
@@ -16,7 +16,9 @@
# under the License.
from .create_dataset import create_dataset
+from .create_dataset_metric import create_dataset_metric
from .create_virtual_dataset import create_virtual_dataset
+from .delete_dataset_metric import delete_dataset_metric
from .get_dataset_info import get_dataset_info
from .list_datasets import list_datasets
from .query_dataset import query_dataset
@@ -24,6 +26,8 @@ from .update_dataset_metric import update_dataset_metric
__all__ = [
"create_dataset",
+ "create_dataset_metric",
+ "delete_dataset_metric",
"create_virtual_dataset",
"get_dataset_info",
"list_datasets",
diff --git a/superset/mcp_service/dataset/tool/create_dataset_metric.py b/superset/mcp_service/dataset/tool/create_dataset_metric.py
new file mode 100644
index 00000000000..6d7e946a371
--- /dev/null
+++ b/superset/mcp_service/dataset/tool/create_dataset_metric.py
@@ -0,0 +1,172 @@
+# 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.
+
+"""Create a saved metric on a dataset (FastMCP tool)."""
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset import security_manager
+from superset.extensions import event_logger
+from superset.mcp_service.dataset.schemas import (
+ CreateDatasetMetricRequest,
+ CreateDatasetMetricResponse,
+)
+from superset.mcp_service.dataset.tool.update_dataset_metric import (
+ _serialize_metric,
+)
+
+
+@tool(
+ tags=["mutate"],
+ class_permission_name="Dataset",
+ method_permission_name="write",
+ annotations=ToolAnnotations(
+ title="Create dataset metric",
+ readOnlyHint=False,
+ destructiveHint=False,
+ idempotentHint=False,
+ openWorldHint=False,
+ ),
+)
+async def create_dataset_metric(
+ request: CreateDatasetMetricRequest, ctx: Context
+) -> CreateDatasetMetricResponse:
+ """Add a saved metric to a dataset identified by ID or UUID.
+
+ Requires metric_name and expression; accepts the same optional properties
+ as update_dataset_metric. Existing metrics are preserved. Requires dataset
+ editorship (or Admin), just like updating a dataset. Use get_dataset_info
+ to inspect existing names before creating a metric.
+ """
+ updates = request.updates()
+ await ctx.info("Creating dataset metric: dataset_id=%s" % (request.dataset_id,))
+
+ try:
+ from sqlalchemy.orm import joinedload, subqueryload
+
+ from superset.commands.dataset.exceptions import (
+ DatasetForbiddenError,
+ DatasetInvalidError,
+ DatasetNotFoundError,
+ DatasetSoftDeletedTwinExistsError,
+ DatasetUpdateFailedError,
+ )
+ from superset.commands.dataset.update import UpdateDatasetCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.exceptions import SupersetSecurityException
+ from superset.mcp_service.dataset.dataset_utils import resolve_dataset
+ from superset.mcp_service.utils.url_utils import get_superset_base_url
+
+ eager_options = [
+ subqueryload(SqlaTable.metrics),
+ joinedload(SqlaTable.database),
+ ]
+
+ with event_logger.log_context(action="mcp.create_dataset_metric.lookup"):
+ dataset = resolve_dataset(request.dataset_id, eager_options)
+
+ if dataset is None:
+ await ctx.warning("Dataset not found: %s" % (request.dataset_id,))
+ return CreateDatasetMetricResponse(
+ error=(
+ f"No dataset found with identifier: {request.dataset_id}."
+ " Use list_datasets to get valid dataset IDs."
+ ),
+ )
+
+ # Check editorship before inspecting metric names. The command repeats
+ # this check and enforces all dataset update validation on persistence.
+ try:
+ security_manager.raise_for_editorship(dataset)
+ except SupersetSecurityException:
+ await ctx.warning(
+ "Dataset metric create forbidden: dataset_id=%s" % (dataset.id,)
+ )
+ return CreateDatasetMetricResponse(
+ error="You must be an owner of this dataset (or an Admin) "
+ "to create its metrics.",
+ )
+
+ metrics = list(dataset.metrics)
+ if any(metric.metric_name == request.metric_name for metric in metrics):
+ await ctx.warning("Metric already exists: %s" % (request.metric_name,))
+ return CreateDatasetMetricResponse(
+ dataset_id=dataset.id,
+ dataset_name=dataset.table_name,
+ error=f"Metric '{request.metric_name}' already exists on this dataset. "
+ "Choose a unique metric_name or use update_dataset_metric.",
+ )
+
+ # Use an additive write: replaying lookup-time stubs can overwrite a
+ # concurrent rename or delete a metric added by another writer.
+ with event_logger.log_context(action="mcp.create_dataset_metric.create"):
+ updated_dataset = UpdateDatasetCommand(
+ dataset.id,
+ {"metrics": [updates]},
+ preserve_existing_metrics=True,
+ ).run()
+
+ created_metric = next(
+ m for m in updated_dataset.metrics if m.metric_name == request.metric_name
+ )
+ await ctx.info(
+ "Dataset metric created: dataset_id=%s, metric_id=%s"
+ % (updated_dataset.id, created_metric.id)
+ )
+ return CreateDatasetMetricResponse(
+ dataset_id=updated_dataset.id,
+ dataset_name=updated_dataset.table_name,
+ metric=_serialize_metric(created_metric),
+ url=(
+ f"{get_superset_base_url()}/explore/"
+ f"?datasource_type=table&datasource_id={updated_dataset.id}"
+ ),
+ )
+
+ except DatasetNotFoundError:
+ await ctx.warning("Dataset not found: %s" % (request.dataset_id,))
+ return CreateDatasetMetricResponse(
+ error=f"No dataset found with identifier: {request.dataset_id}.",
+ )
+ except DatasetForbiddenError:
+ await ctx.warning(
+ "Dataset metric create forbidden: dataset_id=%s" % (request.dataset_id,)
+ )
+ return CreateDatasetMetricResponse(
+ error="You must be an owner of this dataset (or an Admin) "
+ "to create its metrics.",
+ )
+ except DatasetSoftDeletedTwinExistsError as exc:
+ collision_message: str = str(exc)
+ await ctx.warning("Dataset metric validation failed: %s" % (collision_message,))
+ return CreateDatasetMetricResponse(error=collision_message)
+ except DatasetInvalidError as exc:
+ messages = exc.normalized_messages()
+ await ctx.warning("Dataset metric validation failed: %s" % (messages,))
+ return CreateDatasetMetricResponse(error=str(messages))
+ except DatasetUpdateFailedError as exc:
+ await ctx.error("Dataset metric create failed: %s" % (str(exc),))
+ return CreateDatasetMetricResponse(
+ error=f"Failed to create metric: {exc}",
+ )
+ except Exception as exc:
+ await ctx.error(
+ "Unexpected error creating dataset metric: %s: %s"
+ % (type(exc).__name__, str(exc))
+ )
+ raise
diff --git a/superset/mcp_service/dataset/tool/delete_dataset_metric.py b/superset/mcp_service/dataset/tool/delete_dataset_metric.py
new file mode 100644
index 00000000000..ee921146821
--- /dev/null
+++ b/superset/mcp_service/dataset/tool/delete_dataset_metric.py
@@ -0,0 +1,246 @@
+# 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.
+
+"""Delete a saved metric on a dataset (FastMCP tool)."""
+
+from typing import Any
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset import security_manager
+from superset.extensions import event_logger
+from superset.mcp_service.dataset.schemas import (
+ DeleteDatasetMetricRequest,
+ DeleteDatasetMetricResponse,
+ MetricChartReference,
+)
+from superset.mcp_service.dataset.tool.update_dataset_metric import (
+ _find_metric,
+ _metric_not_found_message,
+ _serialize_metric,
+)
+
+
+def _references_metric(value: Any, metric_name: str) -> bool:
+ """Inspect metric controls, HAVING filters, and query-context metric names.
+
+ Only named metric references count, not SQL expressions, adhoc labels,
+ column names, or arbitrary strings elsewhere in the chart configuration.
+ """
+ if isinstance(value, list):
+ return any(_references_metric(item, metric_name) for item in value)
+ if not isinstance(value, dict):
+ return False
+ if value.get("saved_metric") is True and value.get("name") == metric_name:
+ return True
+ if value.get("clause") == "HAVING" and value.get("subject") == metric_name:
+ return True
+ for key, item in value.items():
+ if {"metric", "metrics"}.intersection(key.split("_")):
+ candidates = item if isinstance(item, list) else [item]
+ if any(candidate == metric_name for candidate in candidates):
+ return True
+ if (
+ key == "orderby"
+ and isinstance(item, list)
+ and any(
+ isinstance(order, list) and order and order[0] == metric_name
+ for order in item
+ )
+ ):
+ return True
+ if _references_metric(item, metric_name):
+ return True
+ return False
+
+
+def _find_affected_charts(
+ dataset_id: int, metric_name: str
+) -> list[MetricChartReference]:
+ """Return only accessible charts on this dataset referencing the metric."""
+ from superset import db
+ from superset.exceptions import SupersetSecurityException
+ from superset.models.slice import Slice
+ from superset.utils import json
+ from superset.utils.core import DatasourceType
+
+ charts = (
+ db.session.query(Slice)
+ .filter(
+ Slice.datasource_id == dataset_id,
+ Slice.datasource_type == DatasourceType.TABLE,
+ )
+ .order_by(Slice.id)
+ .all()
+ )
+ references = []
+ for chart in charts:
+ try:
+ security_manager.raise_for_access(chart=chart)
+ except SupersetSecurityException:
+ continue
+ configs = [chart.form_data]
+ if chart.query_context:
+ try:
+ configs.append(json.loads(chart.query_context))
+ except json.JSONDecodeError:
+ # Chart impact is advisory; still inspect valid form data.
+ pass
+ if _references_metric(configs, metric_name):
+ references.append(
+ MetricChartReference(
+ id=chart.id,
+ uuid=str(chart.uuid) if chart.uuid else None,
+ slice_name=chart.slice_name,
+ )
+ )
+ return references
+
+
+@tool(
+ tags=["mutate"],
+ class_permission_name="Dataset",
+ method_permission_name="write",
+ annotations=ToolAnnotations(
+ title="Delete dataset metric",
+ readOnlyHint=False,
+ destructiveHint=True,
+ idempotentHint=False,
+ openWorldHint=False,
+ ),
+)
+async def delete_dataset_metric(
+ request: DeleteDatasetMetricRequest, ctx: Context
+) -> DeleteDatasetMetricResponse:
+ """Delete a saved metric by ID, UUID, or metric_name.
+
+ Requires dataset editorship (or Admin). Other metrics are preserved.
+ Returns accessible charts that reference the deleted metric by name so
+ callers can warn users and repair those charts. Chart definitions are not
+ modified. Inspect the dataset with get_dataset_info before deleting.
+ """
+ await ctx.info("Deleting dataset metric: dataset_id=%s" % (request.dataset_id,))
+
+ try:
+ from sqlalchemy.orm import joinedload, subqueryload
+
+ from superset.commands.dataset.exceptions import (
+ DatasetForbiddenError,
+ DatasetInvalidError,
+ DatasetNotFoundError,
+ DatasetSoftDeletedTwinExistsError,
+ DatasetUpdateFailedError,
+ )
+ from superset.commands.dataset.update import UpdateDatasetCommand
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.exceptions import SupersetSecurityException
+ from superset.mcp_service.dataset.dataset_utils import resolve_dataset
+
+ eager_options = [
+ subqueryload(SqlaTable.metrics),
+ joinedload(SqlaTable.database),
+ ]
+
+ with event_logger.log_context(action="mcp.delete_dataset_metric.lookup"):
+ dataset = resolve_dataset(request.dataset_id, eager_options)
+
+ if dataset is None:
+ await ctx.warning("Dataset not found: %s" % (request.dataset_id,))
+ return DeleteDatasetMetricResponse(
+ error=(
+ f"No dataset found with identifier: {request.dataset_id}."
+ " Use list_datasets to get valid dataset IDs."
+ ),
+ )
+
+ # Check editorship before inspecting metric names. The command repeats
+ # this check and enforces all dataset update validation on persistence.
+ try:
+ security_manager.raise_for_editorship(dataset)
+ except SupersetSecurityException:
+ await ctx.warning(
+ "Dataset metric delete forbidden: dataset_id=%s" % (dataset.id,)
+ )
+ return DeleteDatasetMetricResponse(
+ error="You must be an owner of this dataset (or an Admin) "
+ "to delete its metrics.",
+ )
+
+ metrics = list(dataset.metrics)
+ target = _find_metric(metrics, request.metric)
+ if target is None:
+ message = _metric_not_found_message(metrics, request.metric)
+ await ctx.warning("Metric not found: %s" % (request.metric,))
+ return DeleteDatasetMetricResponse(
+ dataset_id=dataset.id,
+ dataset_name=dataset.table_name,
+ error=message,
+ )
+
+ target_id = target.id
+ deleted_metric = _serialize_metric(target)
+ with event_logger.log_context(action="mcp.delete_dataset_metric.references"):
+ affected_charts = _find_affected_charts(dataset.id, target.metric_name)
+ with event_logger.log_context(action="mcp.delete_dataset_metric.delete"):
+ updated_dataset = UpdateDatasetCommand(
+ dataset.id, {}, delete_metric_ids={target_id}
+ ).run()
+
+ await ctx.info(
+ "Dataset metric deleted: dataset_id=%s, metric_id=%s"
+ % (updated_dataset.id, deleted_metric.id)
+ )
+ return DeleteDatasetMetricResponse(
+ dataset_id=updated_dataset.id,
+ dataset_name=updated_dataset.table_name,
+ metric=deleted_metric,
+ affected_charts=affected_charts,
+ )
+
+ except DatasetNotFoundError:
+ await ctx.warning("Dataset not found: %s" % (request.dataset_id,))
+ return DeleteDatasetMetricResponse(
+ error=f"No dataset found with identifier: {request.dataset_id}.",
+ )
+ except DatasetForbiddenError:
+ await ctx.warning(
+ "Dataset metric delete forbidden: dataset_id=%s" % (request.dataset_id,)
+ )
+ return DeleteDatasetMetricResponse(
+ error="You must be an owner of this dataset (or an Admin) "
+ "to delete its metrics.",
+ )
+ except DatasetSoftDeletedTwinExistsError as exc:
+ collision_message: str = str(exc)
+ await ctx.warning("Dataset metric validation failed: %s" % (collision_message,))
+ return DeleteDatasetMetricResponse(error=collision_message)
+ except DatasetInvalidError as exc:
+ messages = exc.normalized_messages()
+ await ctx.warning("Dataset metric validation failed: %s" % (messages,))
+ return DeleteDatasetMetricResponse(error=str(messages))
+ except DatasetUpdateFailedError as exc:
+ await ctx.error("Dataset metric delete failed: %s" % (str(exc),))
+ return DeleteDatasetMetricResponse(
+ error=f"Failed to delete metric: {exc}",
+ )
+ except Exception as exc:
+ await ctx.error(
+ "Unexpected error deleting dataset metric: %s: %s"
+ % (type(exc).__name__, str(exc))
+ )
+ raise
diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py
index 34583723de9..1341b8231fb 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -368,10 +368,12 @@ MCP_CACHE_CONFIG: dict[str, Any] = {
"add_chart_to_existing_dashboard",
"apply_dashboard_filters",
"create_dataset",
+ "create_dataset_metric",
"create_theme",
"create_virtual_dataset",
"delete_chart",
"delete_dashboard",
+ "delete_dataset_metric",
"duplicate_dashboard",
"execute_sql",
"generate_chart",
diff --git a/superset/mcp_service/utils/response_size_utils.py b/superset/mcp_service/utils/response_size_utils.py
index 96907b8ef7f..be0bc31c6ac 100644
--- a/superset/mcp_service/utils/response_size_utils.py
+++ b/superset/mcp_service/utils/response_size_utils.py
@@ -434,10 +434,12 @@ COMMITTED_WRITE_SPECS: Dict[str, CommittedWriteSpec] = {
"add_chart_to_existing_dashboard": _spec("dashboard", "dashboard"),
"apply_dashboard_filters": _spec("dashboard"),
"create_dataset": _spec("dataset"),
+ "create_dataset_metric": _spec("dataset", "metric"),
"create_theme": _spec("theme", reports_success=True),
"create_virtual_dataset": _spec("dataset"),
"delete_chart": _spec("chart", reports_success=True),
"delete_dashboard": _spec("dashboard", reports_success=True),
+ "delete_dataset_metric": _spec("dataset", "metric"),
"duplicate_dashboard": _spec("dashboard", "dashboard"),
"generate_chart": _spec("chart", "chart", reports_success=True),
"generate_dashboard": _spec("dashboard", "dashboard"),
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_dataset_metric_mutations.py b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_metric_mutations.py
new file mode 100644
index 00000000000..076fed3df10
--- /dev/null
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_dataset_metric_mutations.py
@@ -0,0 +1,529 @@
+# 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.
+
+"""Creation and deletion exercise the registered tools and dataset update path."""
+
+from collections.abc import Iterator
+from typing import Any
+from unittest.mock import MagicMock, patch, PropertyMock
+
+import pytest
+from fastmcp import Client
+from pydantic import ValidationError
+from sqlalchemy.orm import Session
+
+from superset.commands.dataset.exceptions import (
+ DatasetForbiddenError,
+ DatasetInvalidError,
+ DatasetMetricsDuplicateValidationError,
+ DatasetNotFoundError,
+ DatasetSoftDeletedTwinExistsError,
+ DatasetUpdateFailedError,
+)
+from superset.connectors.sqla.models import SqlaTable, SqlMetric
+from superset.exceptions import SupersetSecurityException
+from superset.mcp_service.app import mcp
+from superset.mcp_service.dataset.schemas import CreateDatasetMetricRequest
+from superset.mcp_service.dataset.tool.delete_dataset_metric import (
+ _find_affected_charts,
+ _references_metric,
+)
+from superset.models.core import Database
+from superset.models.slice import Slice
+from superset.utils import json
+
+TOOLS = ["create_dataset_metric", "delete_dataset_metric"]
+UUID = "a1b2c3d4-5678-90ab-cdef-1234567890ab"
+
+
+@pytest.fixture(autouse=True)
+def mock_auth() -> Iterator[None]:
+ """Authenticate an MCP caller without testing authentication itself."""
+ with patch("superset.mcp_service.auth.get_user_from_request") as get_user:
+ get_user.return_value = MagicMock(id=1, username="admin")
+ yield
+
+
+@pytest.fixture(autouse=True)
+def editorship() -> Iterator[MagicMock]:
+ """Permit editorship except in tests explicitly denying it."""
+ with patch(
+ "superset.security.SupersetSecurityManager.raise_for_editorship"
+ ) as check:
+ yield check
+
+
+@pytest.fixture
+def dataset() -> MagicMock:
+ """Supply existing metrics whose metadata must survive mutations."""
+ dataset = MagicMock(id=1, table_name="sales")
+ dataset.metrics = [
+ SqlMetric(id=10, metric_name="revenue", expression="SUM(amount)", uuid=UUID),
+ SqlMetric(
+ id=11, metric_name="count", expression="COUNT(*)", description="keep"
+ ),
+ ]
+ return dataset
+
+
+@pytest.fixture
+def references() -> Iterator[MagicMock]:
+ """Avoid database access in tool orchestration tests."""
+ with patch(
+ "superset.mcp_service.dataset.tool.delete_dataset_metric._find_affected_charts",
+ return_value=[],
+ ) as lookup:
+ yield lookup
+
+
+def request_for(tool: str) -> dict[str, Any]:
+ """Build a minimal valid request for each mutation."""
+ if tool == "create_dataset_metric":
+ return {"dataset_id": 1, "metric_name": "profit", "expression": "SUM(profit)"}
+ return {"dataset_id": 1, "metric": "revenue"}
+
+
+async def call(tool: str, request: dict[str, Any]) -> dict[str, Any]:
+ """Call the registered tool through MCP and decode its response."""
+ async with Client(mcp) as client:
+ result = await client.call_tool(tool, {"request": request})
+ return json.loads(result.content[0].text)
+
+
+@pytest.mark.parametrize("field", ["metric_name", "expression"])
+@pytest.mark.parametrize("value", [None, "", " "])
+def test_create_rejects_blank_required_properties(field: str, value: Any) -> None:
+ """Names and SQL expressions cannot be null, empty, or whitespace."""
+ request = request_for(TOOLS[0])
+ request[field] = value
+ with pytest.raises(ValidationError):
+ CreateDatasetMetricRequest.model_validate(request)
+
+
+@pytest.mark.parametrize("field", ["metric_name", "expression"])
+def test_create_requires_properties(field: str) -> None:
+ """Both name and expression are mandatory."""
+ request = request_for(TOOLS[0])
+ del request[field]
+ with pytest.raises(ValidationError):
+ CreateDatasetMetricRequest.model_validate(request)
+
+
+def test_create_rejects_invalid_extra() -> None:
+ """Reuse the update request's extra JSON validation."""
+ with pytest.raises(ValidationError, match="extra must be a valid JSON"):
+ CreateDatasetMetricRequest.model_validate(
+ {**request_for(TOOLS[0]), "extra": "{"}
+ )
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("identifier", [1, "1", UUID])
+async def test_create_success(dataset: MagicMock, identifier: int | str) -> None:
+ """All optional properties pass through without replaying existing metrics."""
+ request = {
+ **request_for(TOOLS[0]),
+ "dataset_id": identifier,
+ "verbose_name": "Profit",
+ "description": "Exact <UNTRUSTED-CONTENT> text",
+ "d3format": ",.2f",
+ "currency": {"symbol": "USD", "symbolPosition": "prefix"},
+ "warning_text": "Estimated",
+ "metric_type": "sum",
+ "extra": '{"key": "value"}',
+ }
+ properties = {key: value for key, value in request.items() if key != "dataset_id"}
+ created = SqlMetric(id=12, uuid=UUID, **properties)
+ updated = MagicMock(id=1, table_name="sales", metrics=[*dataset.metrics, created])
+ with (
+ patch(
+ "superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset
+ ) as find,
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ command.return_value.run.return_value = updated
+ result = await call(TOOLS[0], request)
+ assert find.call_args.args == (1 if identifier != UUID else UUID,)
+ if identifier == UUID:
+ assert find.call_args.kwargs["id_column"] == "uuid"
+ command.assert_called_once_with(
+ 1, {"metrics": [properties]}, preserve_existing_metrics=True
+ )
+ assert result["error"] is None
+ assert result["metric"] == {"id": 12, "uuid": UUID, **properties}
+ assert result["dataset_id"] == 1
+ assert "datasource_id=1" in result["url"]
+
+
+@pytest.mark.asyncio
+async def test_create_duplicate(dataset: MagicMock) -> None:
+ """Duplicate names produce actionable errors without invoking persistence."""
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ result = await call(
+ TOOLS[0], {**request_for(TOOLS[0]), "metric_name": "revenue"}
+ )
+ command.assert_not_called()
+ assert result["metric"] is None
+ assert "'revenue' already exists" in result["error"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("identifier", [10, "10", UUID.upper(), "revenue"])
+async def test_delete_success(
+ dataset: MagicMock, references: MagicMock, identifier: int | str
+) -> None:
+ """Delete by any supported identifier and report referencing charts."""
+ references.return_value = [{"id": 5, "uuid": UUID, "slice_name": "Revenue"}]
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ command.return_value.run.return_value = dataset
+ result = await call(TOOLS[1], {"dataset_id": UUID, "metric": identifier})
+ command.assert_called_once_with(1, {}, delete_metric_ids={10})
+ references.assert_called_once_with(1, "revenue")
+ assert result["error"] is None
+ assert result["metric"]["id"] == 10
+ assert result["metric"]["metric_name"] == "revenue"
+ assert result["affected_charts"] == references.return_value
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("empty", [False, True])
+async def test_delete_unknown_metric(dataset: MagicMock, empty: bool) -> None:
+ """Reuse did-you-mean guidance, including the empty dataset case."""
+ if empty:
+ dataset.metrics = []
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ result = await call(TOOLS[1], {"dataset_id": 1, "metric": "revenu"})
+ command.assert_not_called()
+ assert result["metric"] is None
+ assert "not found" in result["error"]
+ assert ("no saved metrics" if empty else "Did you mean: revenue?") in result[
+ "error"
+ ]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("tool", TOOLS)
+async def test_unknown_dataset(tool: str) -> None:
+ """An unknown dataset yields a clear error and no writes."""
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=None),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ result = await call(tool, request_for(tool))
+ command.assert_not_called()
+ assert "No dataset found" in result["error"]
+ assert result["metric"] is None
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("tool", TOOLS)
+async def test_non_editor_cannot_inspect_metrics(
+ tool: str, dataset: MagicMock, editorship: MagicMock
+) -> None:
+ """Deny before duplicate checks, name suggestions, or chart inspection."""
+ editorship.side_effect = SupersetSecurityException(MagicMock(message="denied"))
+ type(dataset).metrics = PropertyMock(side_effect=AssertionError("metrics accessed"))
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ result = await call(tool, request_for(tool))
+ command.assert_not_called()
+ assert "owner" in result["error"]
+ assert "revenue" not in result["error"]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("tool", TOOLS)
+@pytest.mark.parametrize(
+ "exception, message",
+ [
+ (DatasetNotFoundError(), "No dataset found"),
+ (DatasetForbiddenError(), "owner"),
+ (DatasetUpdateFailedError(), "Failed to"),
+ (DatasetSoftDeletedTwinExistsError(UUID), f"/api/v1/dataset/{UUID}/restore"),
+ (
+ DatasetInvalidError(exceptions=[DatasetMetricsDuplicateValidationError()]),
+ "duplicate",
+ ),
+ ],
+)
+async def test_command_errors(
+ tool: str,
+ exception: Exception,
+ message: str,
+ dataset: MagicMock,
+ references: MagicMock,
+) -> None:
+ """Preserve the existing update path's command validation and failures."""
+ with (
+ patch("superset.daos.dataset.DatasetDAO.find_by_id", return_value=dataset),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as command,
+ ):
+ command.return_value.run.side_effect = exception
+ result = await call(tool, request_for(tool))
+ assert result["metric"] is None
+ assert message in result["error"]
+ assert not result.get("affected_charts")
+
+
+@pytest.mark.parametrize(
+ "config, expected",
+ [
+ ({"metric": "revenue"}, True),
+ ({"metrics": ["count", "revenue"]}, True),
+ ({"metrics_b": ["revenue"]}, True),
+ ({"timeseries_limit_metric": "revenue"}, True),
+ ({"matrixify_topn_metric_rows": "revenue"}, True),
+ ({"queries": [{"metrics": ["revenue"]}]}, True),
+ ({"metric": {"saved_metric": True, "name": "revenue"}}, True),
+ ({"orderby": [["revenue", False]]}, True),
+ ({"adhoc_filters": [{"clause": "HAVING", "subject": "revenue"}]}, True),
+ ({"metrics": [{"label": "revenue", "sqlExpression": "SUM(amount)"}]}, False),
+ ({"groupby": ["revenue"], "title": "revenue"}, False),
+ ({"metrics": ["net_revenue"]}, False),
+ ({"adhoc_filters": [{"clause": "WHERE", "subject": "revenue"}]}, False),
+ ({}, False),
+ ],
+)
+def test_metric_references(config: dict[str, Any], expected: bool) -> None:
+ """Detect named references without treating labels or columns as metrics."""
+ assert _references_metric(config, "revenue") is expected
+
+
+def test_reference_lookup_is_dataset_scoped_and_access_checked(
+ session: Session,
+) -> None:
+ """Do not expose inaccessible or unrelated charts in the impact report."""
+ Database.metadata.create_all(session.bind)
+ charts: list[Slice] = [
+ Slice(
+ slice_name="Match",
+ datasource_id=1,
+ datasource_type="table",
+ params='{"metric":"revenue"}',
+ ),
+ Slice(
+ slice_name="Hidden",
+ datasource_id=1,
+ datasource_type="table",
+ params='{"metric":"revenue"}',
+ ),
+ Slice(
+ slice_name="Other dataset",
+ datasource_id=2,
+ datasource_type="table",
+ params='{"metric":"revenue"}',
+ ),
+ Slice(
+ slice_name="Other type",
+ datasource_id=1,
+ datasource_type="druid",
+ params='{"metric":"revenue"}',
+ ),
+ Slice(
+ slice_name="Other metric",
+ datasource_id=1,
+ datasource_type="table",
+ params='{"metric":"count"}',
+ ),
+ Slice(
+ slice_name="Query context",
+ datasource_id=1,
+ datasource_type="table",
+ params="{}",
+ query_context='{"queries":[{"metrics":["revenue"]}]}',
+ ),
+ ]
+ session.add_all(charts)
+ session.commit()
+
+ def check_access(*, chart: Slice) -> None:
+ """Deny just one of the matching charts."""
+ if chart.slice_name == "Hidden":
+ raise SupersetSecurityException(MagicMock(message="denied"))
+
+ with patch(
+ "superset.security.SupersetSecurityManager.raise_for_access",
+ side_effect=check_access,
+ ) as check:
+ result = _find_affected_charts(1, "revenue")
+ assert [chart.slice_name for chart in result] == ["Match", "Query context"]
+ expected_access_checks: int = sum(
+ chart.datasource_id == 1 and chart.datasource_type == "table"
+ for chart in charts
+ )
+ assert check.call_count == expected_access_checks
+
+
+@pytest.mark.asyncio
+async def test_real_command_create_and_delete(session: Session) -> None:
+ """Persist new metrics and delete even the last one through the real command."""
+ from superset.daos.dataset import DatasetDAO
+
+ Database.metadata.create_all(session.bind)
+ database = Database(database_name="metric_mutations", sqlalchemy_uri="sqlite://")
+ original = SqlMetric(metric_name="count", expression="COUNT(*)", description="keep")
+ dataset = SqlaTable(
+ database=database, table_name="metric_mutations", metrics=[original]
+ )
+ session.add(dataset)
+ session.commit()
+ dataset_id, original_id = dataset.id, original.id
+ with (
+ patch.object(DatasetDAO, "base_filter", None),
+ patch("superset.security.SupersetSecurityManager.is_admin", return_value=True),
+ ):
+ created = await call(
+ TOOLS[0], {**request_for(TOOLS[0]), "dataset_id": dataset_id}
+ )
+ assert created["error"] is None
+ created_id = created["metric"]["id"]
+ assert created["metric"]["uuid"]
+ session.expire_all()
+ assert session.get(SqlMetric, created_id).expression == "SUM(profit)"
+ assert session.get(SqlMetric, original_id).description == "keep"
+ for metric_id in [created_id, original_id]:
+ deleted = await call(
+ TOOLS[1], {"dataset_id": dataset_id, "metric": metric_id}
+ )
+ assert deleted["error"] is None
+ session.expire_all()
+ assert session.get(SqlMetric, metric_id) is None
+ assert session.get(SqlaTable, dataset_id).metrics == []
+
+
+@pytest.mark.asyncio
+async def test_create_preserves_metric_changes_after_lookup(session: Session) -> None:
+ """A concurrent addition and rename must survive the create command."""
+ from superset.daos.dataset import DatasetDAO
+
+ Database.metadata.create_all(session.bind)
+ database = Database(database_name="concurrent_metrics", sqlalchemy_uri="sqlite://")
+ original = SqlMetric(metric_name="count", expression="COUNT(*)")
+ dataset = SqlaTable(
+ database=database, table_name="concurrent_metrics", metrics=[original]
+ )
+ session.add(dataset)
+ session.commit()
+ dataset_id: int = dataset.id
+ original_id: int = original.id
+ find_by_id = DatasetDAO.find_by_id
+ lookup_count: int = 0
+ added = SqlMetric(metric_name="concurrent", expression="SUM(amount)")
+
+ def find_with_concurrent_change(*args: Any, **kwargs: Any) -> SqlaTable | None:
+ """Commit another writer's changes between tool lookup and validation."""
+ nonlocal lookup_count
+ lookup_count += 1
+ if lookup_count == 2:
+ original.metric_name = "renamed"
+ dataset.metrics.append(added)
+ session.commit()
+ return find_by_id(*args, **kwargs)
+
+ with (
+ patch.object(DatasetDAO, "base_filter", None),
+ patch.object(DatasetDAO, "find_by_id", side_effect=find_with_concurrent_change),
+ patch("superset.security.SupersetSecurityManager.is_admin", return_value=True),
+ ):
+ result = await call(
+ TOOLS[0], {**request_for(TOOLS[0]), "dataset_id": dataset_id}
+ )
+ assert result["error"] is None
+ session.expire_all()
+ assert session.get(SqlMetric, original_id).metric_name == "renamed"
+ assert {
+ metric.metric_name for metric in session.get(SqlaTable, dataset_id).metrics
+ } == {"renamed", "concurrent", "profit"}
+
+
+@pytest.mark.parametrize("params", ['{"metric":"revenue"}', "{}"])
+def test_reference_lookup_tolerates_malformed_query_context(
+ session: Session, params: str
+) -> None:
+ """Malformed advisory JSON must not hide form-data or later chart matches."""
+ Database.metadata.create_all(session.bind)
+ charts: list[Slice] = [
+ Slice(
+ slice_name="Malformed",
+ datasource_id=1,
+ datasource_type="table",
+ params=params,
+ query_context="{invalid",
+ ),
+ Slice(
+ slice_name="Valid",
+ datasource_id=1,
+ datasource_type="table",
+ params="{}",
+ query_context='{"queries":[{"metrics":["revenue"]}]}',
+ ),
+ ]
+ session.add_all(charts)
+ session.commit()
+ with patch("superset.security.SupersetSecurityManager.raise_for_access"):
+ result = _find_affected_charts(1, "revenue")
+ expected: list[str] = ["Malformed", "Valid"] if "revenue" in params else ["Valid"]
+ assert [chart.slice_name for chart in result] == expected
+
+
+@pytest.mark.asyncio
+async def test_delete_preserves_metric_added_after_lookup(
+ session: Session, references: MagicMock
+) -> None:
+ """A metric committed during chart lookup must survive targeted deletion."""
+ from superset.daos.dataset import DatasetDAO
+
+ Database.metadata.create_all(session.bind)
+ database = Database(database_name="concurrent_delete", sqlalchemy_uri="sqlite://")
+ target = SqlMetric(metric_name="revenue", expression="SUM(amount)")
+ dataset = SqlaTable(
+ database=database, table_name="concurrent_delete", metrics=[target]
+ )
+ session.add(dataset)
+ session.commit()
+ dataset_id, target_id = dataset.id, target.id
+ added = SqlMetric(metric_name="concurrent", expression="COUNT(*)")
+
+ def add_metric(*args: Any) -> list[Any]:
+ """Commit a competing write after the tool has captured its metric list."""
+ dataset.metrics.append(added)
+ session.commit()
+ return []
+
+ references.side_effect = add_metric
+ with (
+ patch.object(DatasetDAO, "base_filter", None),
+ patch("superset.security.SupersetSecurityManager.is_admin", return_value=True),
+ ):
+ result = await call(TOOLS[1], {"dataset_id": dataset_id, "metric": target_id})
+ assert result["error"] is None
+ session.expire_all()
+ assert session.get(SqlMetric, target_id) is None
+ assert {
+ metric.metric_name for metric in session.get(SqlaTable, dataset_id).metrics
+ } == {"concurrent"}
diff --git a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
index 76f08979740..64467ac18b2 100644
--- a/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
+++ b/tests/unit_tests/mcp_service/test_mcp_tool_registration.py
@@ -38,10 +38,12 @@ EXPECTED_TOOL_OUTPUT_FIELDS = {
"add_chart_to_existing_dashboard": "dashboard_url",
"apply_dashboard_filters": "permalink_key",
"create_dataset": "table_name",
+ "create_dataset_metric": "metric",
"create_theme": "theme_name",
"create_virtual_dataset": "dataset_name",
"delete_chart": "deleted_id",
"delete_dashboard": "deleted_id",
+ "delete_dataset_metric": "affected_charts",
"duplicate_dashboard": "duplicated_slices",
"execute_sql": "statements",
"find_users": "users",
@@ -123,10 +125,12 @@ MUTATING_TOOLS = {
"add_chart_to_existing_dashboard",
"apply_dashboard_filters",
"create_dataset",
+ "create_dataset_metric",
"create_theme",
"create_virtual_dataset",
"delete_chart",
"delete_dashboard",
+ "delete_dataset_metric",
"duplicate_dashboard",
"execute_sql",
"generate_chart",
@@ -162,6 +166,7 @@ NON_COMMITTING_MUTATING_TOOLS = {
DESTRUCTIVE_TOOLS = {
"delete_chart",
"delete_dashboard",
+ "delete_dataset_metric",
"execute_sql",
"manage_dashboard_owners",
"manage_dashboard_roles",
diff --git a/tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py b/tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py
index 77b82cc84a7..723b5a2b4f8 100644
--- a/tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py
+++ b/tests/unit_tests/mcp_service/test_rbac_tool_enforcement.py
@@ -110,6 +110,18 @@ _MUTATING_TOOLS: list[tuple[str, dict[str, Any], str, str]] = [
"write",
"Dataset",
),
+ (
+ "create_dataset_metric",
+ {"dataset_id": 1, "metric_name": "count", "expression": "COUNT(*)"},
+ "write",
+ "Dataset",
+ ),
+ (
+ "delete_dataset_metric",
+ {"dataset_id": 1, "metric": "count"},
+ "write",
+ "Dataset",
+ ),
(
"create_theme",
{"theme_name": "Denied Theme", "json_data": {"token": {}}},
diff --git a/tests/unit_tests/mcp_service/test_tool_inventory.py b/tests/unit_tests/mcp_service/test_tool_inventory.py
index 03a451eaf9a..a723344298e 100644
--- a/tests/unit_tests/mcp_service/test_tool_inventory.py
+++ b/tests/unit_tests/mcp_service/test_tool_inventory.py
@@ -36,10 +36,12 @@ TOOL_BUDGETS = {
"add_chart_to_existing_dashboard": 1_500,
"apply_dashboard_filters": 2_900,
"create_dataset": 1_800,
+ "create_dataset_metric": 2_800,
"create_theme": 1_100,
"create_virtual_dataset": 3_700,
"delete_chart": 1_100,
"delete_dashboard": 1_100,
+ "delete_dataset_metric": 1_300,
"duplicate_dashboard": 1_900,
"execute_sql": 2_100,
"find_users": 1_500,