You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/10/07 07:14:42 UTC
(superset) branch master updated: fix(mcp): state that dataset tools are SQL-only and point to semantic tools (#44994)
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi 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 ec1688facd7 fix(mcp): state that dataset tools are SQL-only and point to semantic tools (#44994)
ec1688facd7 is described below
commit ec1688facd7b5b33a7aa98f887e4228204b5fdcd
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Wed Oct 7 01:14:34 2026 -0600
fix(mcp): state that dataset tools are SQL-only and point to semantic tools (#44994)
Co-authored-by: Mike Bridge <mi...@ext.preset.io>
---
docs/admin_docs/configuration/mcp-server.mdx | 14 ++
superset/mcp_service/dataset/dataset_utils.py | 5 +
.../mcp_service/dataset/tool/create_dataset.py | 11 +-
.../mcp_service/dataset/tool/get_dataset_info.py | 7 +-
superset/mcp_service/dataset/tool/list_datasets.py | 11 +-
superset/mcp_service/dataset/tool/query_dataset.py | 16 +-
.../mcp_service/dataset/tool/update_dataset.py | 16 +-
.../dataset/tool/test_sql_dataset_guidance.py | 181 +++++++++++++++++++++
.../unit_tests/mcp_service/test_tool_inventory.py | 32 ++++
9 files changed, 278 insertions(+), 15 deletions(-)
diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx
index 57b58e22577..eefe60bcc53 100644
--- a/docs/admin_docs/configuration/mcp-server.mdx
+++ b/docs/admin_docs/configuration/mcp-server.mdx
@@ -1700,6 +1700,20 @@ structured errors instead of apparently successful previews, charts or exports.
## Dataset discovery and routing
+`list_datasets`, `get_dataset_info`, `update_dataset`, and `query_dataset`
+operate on registered **SQL datasets** (physical tables or virtual SQL queries),
+not external semantic views. `create_dataset` registers a physical table as a
+SQL dataset; it does not create or register semantic views. The `certified`
+filter in `list_datasets` selects certified SQL datasets, not external views.
+
+For existing semantic views, use `list_metrics` to discover metrics and
+`get_table` to query them, retaining the returned source and view identity.
+These tools do not provision or edit semantic views. A dataset not-found error
+offers this routing guidance without establishing whether a semantic view
+exists or is accessible. Numeric IDs can overlap across source types; do not
+reuse a semantic-view ID as a SQL-dataset ID or substitute another source.
+All tool permissions, feature flags, and datasource access checks still apply.
+
`list_datasets` searches table name, **description**, schema, and SQL using
escaped, case-insensitive substring matching. Write descriptions with business
terminology to make differently named datasets discoverable. A complete UUID
diff --git a/superset/mcp_service/dataset/dataset_utils.py b/superset/mcp_service/dataset/dataset_utils.py
index 9a1a14fbb1b..e2f9340b391 100644
--- a/superset/mcp_service/dataset/dataset_utils.py
+++ b/superset/mcp_service/dataset/dataset_utils.py
@@ -21,6 +21,11 @@ from typing import Any
from superset.mcp_service.utils import _is_uuid
+SQL_DATASET_SOURCE_GUIDANCE: str = (
+ "This tool supports SQL datasets only. For semantic views, discover metrics "
+ "with list_metrics and query with get_table."
+)
+
def resolve_dataset(
identifier: int | str, eager_options: list[Any] | None = None
diff --git a/superset/mcp_service/dataset/tool/create_dataset.py b/superset/mcp_service/dataset/tool/create_dataset.py
index 9a3a903973a..dadecd0e4bb 100644
--- a/superset/mcp_service/dataset/tool/create_dataset.py
+++ b/superset/mcp_service/dataset/tool/create_dataset.py
@@ -36,6 +36,7 @@ from superset.commands.dataset.exceptions import (
DatasetSoftDeletedTwinExistsError,
)
from superset.extensions import event_logger
+from superset.mcp_service.dataset.dataset_utils import SQL_DATASET_SOURCE_GUIDANCE
from superset.mcp_service.dataset.schemas import (
CreateDatasetRequest,
DatasetError,
@@ -62,7 +63,10 @@ def _classify_invalid_error(exc: DatasetInvalidError) -> DatasetError:
if "DatasetExistsValidationError" in classnames:
return DatasetError.create(error=str(messages), error_type="DatasetExistsError")
if "TableNotFoundValidationError" in classnames:
- return DatasetError.create(error=str(messages), error_type="TableNotFoundError")
+ return DatasetError.create(
+ error=f"{messages}. {SQL_DATASET_SOURCE_GUIDANCE}",
+ error_type="TableNotFoundError",
+ )
# Other DatasetInvalidError sub-types are returned as generic ValidationError.
# Add explicit branches here when callers need to distinguish them.
return DatasetError.create(error=str(messages), error_type="ValidationError")
@@ -126,7 +130,10 @@ def _build_dataset_properties(
async def create_dataset(
request: CreateDatasetRequest, ctx: Context
) -> DatasetInfo | DatasetError:
- """Register a physical table as a Superset dataset.
+ """Register a physical table as a SQL dataset, not a semantic view.
+
+ For existing semantic views, use list_metrics for discovery and get_table
+ for queries.
Wraps POST /api/v1/dataset/ — the same endpoint the UI uses when you click
Data → Datasets → +Dataset. Returns full dataset metadata (same shape as
diff --git a/superset/mcp_service/dataset/tool/get_dataset_info.py b/superset/mcp_service/dataset/tool/get_dataset_info.py
index 44ce55f5e84..9c4eb499cf8 100644
--- a/superset/mcp_service/dataset/tool/get_dataset_info.py
+++ b/superset/mcp_service/dataset/tool/get_dataset_info.py
@@ -31,6 +31,7 @@ from sqlalchemy.orm import joinedload, subqueryload
from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.extensions import event_logger
+from superset.mcp_service.dataset.dataset_utils import SQL_DATASET_SOURCE_GUIDANCE
from superset.mcp_service.dataset.schemas import (
DatasetError,
DatasetInfo,
@@ -61,7 +62,9 @@ logger = logging.getLogger(__name__)
async def get_dataset_info(
request: GetDatasetInfoRequest, ctx: Context
) -> DatasetInfo | DatasetError:
- """Get dataset metadata by ID or UUID.
+ """Get metadata by ID or UUID for SQL datasets only.
+
+ For semantic views, use list_metrics for discovery and get_table for queries.
Returns columns, metrics, and schema details.
@@ -180,6 +183,8 @@ async def get_dataset_info(
),
)
else:
+ if isinstance(result, DatasetError) and result.error_type == "not_found":
+ result.error = f"{result.error}. {SQL_DATASET_SOURCE_GUIDANCE}"
await ctx.warning(
"Dataset retrieval failed: error_type=%s, error=%s"
% (result.error_type, result.error)
diff --git a/superset/mcp_service/dataset/tool/list_datasets.py b/superset/mcp_service/dataset/tool/list_datasets.py
index d29e1a42c3a..74e9952359f 100644
--- a/superset/mcp_service/dataset/tool/list_datasets.py
+++ b/superset/mcp_service/dataset/tool/list_datasets.py
@@ -98,11 +98,14 @@ async def list_datasets(
] = None,
ctx: Context | None = None,
) -> DatasetList | DatasetError:
- """List/search/filter datasets.
+ """SQL datasets only.
- Returns dataset metadata including table name, schema, and last modified
- time. Set ``request.certified`` to true to return only governed,
- semantic-layer datasets; false returns only uncertified datasets, while
+ For semantic views, use list_metrics for discovery and get_table for queries.
+
+ List/search/filter physical or virtual SQL datasets. Returns dataset metadata
+ including table name, schema, and last modified
+ time. Set ``request.certified`` to true to return only certified SQL
+ datasets; false returns only uncertified datasets, while
omitting it preserves the unfiltered behavior.
Search matches schema, SQL, table name, and description as case-insensitive
diff --git a/superset/mcp_service/dataset/tool/query_dataset.py b/superset/mcp_service/dataset/tool/query_dataset.py
index 9cd12bfabaf..e9868407dba 100644
--- a/superset/mcp_service/dataset/tool/query_dataset.py
+++ b/superset/mcp_service/dataset/tool/query_dataset.py
@@ -18,8 +18,8 @@
"""
MCP tool: query_dataset
-Query a dataset using its semantic layer (saved metrics, calculated columns,
-dimensions) without requiring a saved chart.
+Query a SQL dataset using its saved metrics, calculated columns and
+dimensions without requiring a saved chart.
"""
import logging
@@ -40,7 +40,10 @@ from superset.common.tabular_query import (
from superset.exceptions import OAuth2Error, OAuth2RedirectError, SupersetException
from superset.extensions import event_logger
from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
-from superset.mcp_service.dataset.dataset_utils import resolve_dataset
+from superset.mcp_service.dataset.dataset_utils import (
+ resolve_dataset,
+ SQL_DATASET_SOURCE_GUIDANCE,
+)
from superset.mcp_service.dataset.schemas import (
DatasetError,
QueryDatasetFilter,
@@ -80,11 +83,13 @@ _NO_SAVED_METRICS_HINT = (
async def query_dataset( # noqa: C901
request: QueryDatasetRequest, ctx: Context
) -> QueryDatasetResponse | DatasetError:
- """Query a dataset using its semantic layer (saved metrics, dimensions, filters).
+ """Query SQL datasets only using saved metrics, dimensions and filters.
+
+ For semantic views, use list_metrics for discovery and get_table for queries.
Returns tabular data without requiring a saved chart. Use this when you want
to compute saved metrics, group by dimensions, or apply filters directly
- against a dataset's curated semantic layer.
+ against a SQL dataset's curated metrics and columns.
Metrics must be saved metric names from get_dataset_info; ad-hoc
expressions such as "SUM(col)" are not accepted. When the dataset has no
@@ -159,6 +164,7 @@ async def query_dataset( # noqa: C901
error=(
f"No dataset found with identifier: {request.dataset_id}."
" Use list_datasets to get valid dataset IDs."
+ f" {SQL_DATASET_SOURCE_GUIDANCE}"
),
error_type="NotFound",
)
diff --git a/superset/mcp_service/dataset/tool/update_dataset.py b/superset/mcp_service/dataset/tool/update_dataset.py
index 4b17022f475..b82fa5fb3c1 100644
--- a/superset/mcp_service/dataset/tool/update_dataset.py
+++ b/superset/mcp_service/dataset/tool/update_dataset.py
@@ -29,6 +29,7 @@ from superset_core.mcp.decorators import tool, ToolAnnotations
from superset import security_manager
from superset.exceptions import SupersetException
from superset.extensions import event_logger
+from superset.mcp_service.dataset.dataset_utils import SQL_DATASET_SOURCE_GUIDANCE
from superset.mcp_service.dataset.schemas import (
UpdateDatasetRequest,
UpdateDatasetResponse,
@@ -78,8 +79,13 @@ def _sync_error_message(ex: Exception) -> str:
async def update_dataset( # noqa: C901
request: UpdateDatasetRequest, ctx: Context
) -> UpdateDatasetResponse:
- """Update a dataset's name, SQL, description, default datetime column or
- cache timeout, and optionally re-sync its columns from the data source.
+ """Update SQL datasets only; semantic views are not supported.
+
+ For existing semantic views, use list_metrics for discovery and get_table
+ for queries.
+
+ Update a dataset's name, SQL, description, default datetime column or cache
+ timeout, and optionally re-sync its columns from the data source.
Only the properties you pass are changed. ``sql`` applies to virtual
datasets only. When ``sql`` changes, columns are re-synced from the new
@@ -158,6 +164,7 @@ async def update_dataset( # noqa: C901
error=(
f"No dataset found with identifier: {display_id}."
" Use list_datasets to get valid dataset IDs."
+ f" {SQL_DATASET_SOURCE_GUIDANCE}"
),
)
@@ -315,7 +322,10 @@ async def update_dataset( # noqa: C901
except DatasetNotFoundError:
await ctx.warning("Dataset not found: %s" % (request.dataset_id,))
return UpdateDatasetResponse(
- error=f"No dataset found with identifier: {request.dataset_id}.",
+ error=(
+ f"No dataset found with identifier: {request.dataset_id}. "
+ f"{SQL_DATASET_SOURCE_GUIDANCE}"
+ ),
)
except DatasetForbiddenError:
await ctx.warning(
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_sql_dataset_guidance.py b/tests/unit_tests/mcp_service/dataset/tool/test_sql_dataset_guidance.py
new file mode 100644
index 00000000000..5d702899fe7
--- /dev/null
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_sql_dataset_guidance.py
@@ -0,0 +1,181 @@
+# 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.
+
+"""SQL-dataset errors guide semantic callers without resolving another source."""
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+from fastmcp import Client
+from fastmcp.client.client import CallToolResult
+
+from superset.commands.dataset.exceptions import (
+ DatasetDataAccessIsNotAllowed,
+ DatasetInvalidError,
+ DatasetNotFoundError,
+ TableNotFoundValidationError,
+)
+from superset.mcp_service.app import mcp
+from superset.mcp_service.dataset.schemas import DatasetError
+from superset.mcp_service.dataset.tool.create_dataset import _classify_invalid_error
+from superset.mcp_service.privacy import DATA_MODEL_METADATA_ERROR_TYPE
+from superset.sql.parse import Table
+from superset.utils import json
+
+
+async def _call(name: str, request: dict[str, object]) -> dict[str, object]:
+ """Call the registered tool as an authenticated metadata-enabled user."""
+ with (
+ patch(
+ "superset.mcp_service.auth.get_user_from_request",
+ return_value=MagicMock(id=1),
+ ),
+ patch(
+ "superset.mcp_service.dataset.tool.get_dataset_info.user_can_view_data_model_metadata",
+ return_value=True,
+ ),
+ patch(
+ "superset.mcp_service.dataset.tool.query_dataset.user_can_view_data_model_metadata",
+ return_value=True,
+ ),
+ ):
+ async with Client(mcp) as client:
+ result: CallToolResult = await client.call_tool(name, {"request": request})
+ return json.loads(result.content[0].text)
+
+
+def _assert_guidance(error: object) -> None:
+ """Assert the action and source distinction, not a shared message constant."""
+ assert isinstance(error, str)
+ assert "SQL dataset" in error
+ assert "semantic view" in error
+ assert "list_metrics" in error
+ assert "get_table" in error
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("identifier", [7, "9b6f05a7-3b88-4ad9-a34e-7a9b2ea465f4"])
+async def test_get_dataset_info_missing_source_guidance(identifier: int | str) -> None:
+ """IDs and UUIDs receive generic guidance, not a semantic-view diagnosis."""
+ with patch(
+ "superset.mcp_service.mcp_core.ModelGetInfoCore._find_object", return_value=None
+ ):
+ data: dict[str, object] = await _call(
+ "get_dataset_info", {"identifier": identifier}
+ )
+ assert data["error_type"] == "not_found"
+ _assert_guidance(data["error"])
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("name", ["query_dataset", "update_dataset"])
+@pytest.mark.parametrize("identifier", [7, "9b6f05a7-3b88-4ad9-a34e-7a9b2ea465f4"])
+async def test_missing_sql_dataset_guidance(name: str, identifier: int | str) -> None:
+ """Do not resolve a different source after a SQL-dataset miss."""
+ request: dict[str, object] = {"dataset_id": identifier}
+ if name == "update_dataset":
+ request["description"] = "unchanged"
+ else:
+ request["columns"] = ["category"]
+ lookup_path: str = (
+ "superset.mcp_service.dataset.tool.query_dataset.resolve_dataset"
+ if name == "query_dataset"
+ else "superset.mcp_service.dataset.dataset_utils.resolve_dataset"
+ )
+ with (
+ patch(lookup_path, return_value=None) as lookup,
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as update,
+ ):
+ data: dict[str, object] = await _call(name, request)
+ lookup.assert_called_once()
+ assert lookup.call_args.args[0] == identifier
+ update.assert_not_called()
+ _assert_guidance(data["error"])
+ if name == "query_dataset":
+ assert data["error_type"] == "NotFound"
+
+
+@pytest.mark.asyncio
+async def test_update_dataset_disappearing_source_guidance() -> None:
+ """The command's not-found race carries the same SQL-only guidance."""
+ dataset: MagicMock = MagicMock(id=7, columns=[])
+ with (
+ patch(
+ "superset.mcp_service.dataset.dataset_utils.resolve_dataset",
+ return_value=dataset,
+ ),
+ patch("superset.security.SupersetSecurityManager.raise_for_editorship"),
+ patch("superset.commands.dataset.update.UpdateDatasetCommand") as update,
+ ):
+ update.return_value.run.side_effect = DatasetNotFoundError()
+ data: dict[str, object] = await _call(
+ "update_dataset", {"dataset_id": 7, "description": "x"}
+ )
+ _assert_guidance(data["error"])
+
+
+def test_create_dataset_missing_table_guidance() -> None:
+ """A missing physical table can be a mistaken semantic-view registration."""
+ result: DatasetError = _classify_invalid_error(
+ DatasetInvalidError(exceptions=[TableNotFoundValidationError(Table("orders"))])
+ )
+ assert result.error_type == "TableNotFoundError"
+ assert "orders" in result.error
+ _assert_guidance(result.error)
+
+
+def test_create_dataset_denial_does_not_suggest_an_alternative_source() -> None:
+ """Access denials remain denials, not routing suggestions."""
+ result: DatasetError = _classify_invalid_error(
+ DatasetInvalidError(exceptions=[DatasetDataAccessIsNotAllowed("denied")])
+ )
+ assert result.error_type == "AccessDeniedError"
+ assert result.error == "Access denied"
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("name", ["get_dataset_info", "query_dataset"])
+async def test_privacy_denial_precedes_lookup_and_guidance(name: str) -> None:
+ """Routing advice must not bypass the direct-call privacy gate."""
+ request: dict[str, object] = {
+ "identifier" if name == "get_dataset_info" else "dataset_id": 7
+ }
+ if name == "query_dataset":
+ request["columns"] = ["category"]
+ with (
+ patch(
+ "superset.mcp_service.auth.get_user_from_request",
+ return_value=MagicMock(id=1),
+ ),
+ patch(
+ f"superset.mcp_service.dataset.tool.{name}.user_can_view_data_model_metadata",
+ return_value=False,
+ ),
+ patch(
+ "superset.mcp_service.mcp_core.ModelGetInfoCore._find_object"
+ ) as info_lookup,
+ patch(
+ "superset.mcp_service.dataset.tool.query_dataset.resolve_dataset"
+ ) as dataset_lookup,
+ ):
+ async with Client(mcp) as client:
+ result: CallToolResult = await client.call_tool(name, {"request": request})
+ data: dict[str, object] = json.loads(result.content[0].text)
+ assert data["error_type"] == DATA_MODEL_METADATA_ERROR_TYPE
+ assert "list_metrics" not in str(data["error"])
+ info_lookup.assert_not_called()
+ dataset_lookup.assert_not_called()
diff --git a/tests/unit_tests/mcp_service/test_tool_inventory.py b/tests/unit_tests/mcp_service/test_tool_inventory.py
index 84afacb0b8d..f484196d8b5 100644
--- a/tests/unit_tests/mcp_service/test_tool_inventory.py
+++ b/tests/unit_tests/mcp_service/test_tool_inventory.py
@@ -21,6 +21,7 @@ from copy import deepcopy
from typing import Any
import pytest
+from fastmcp.tools.tool import Tool
from jsonschema import Draft202012Validator
from superset.mcp_service.app import mcp
@@ -149,6 +150,37 @@ async def test_inventory_budgets_cover_every_registered_tool() -> None:
assert {tool.name for tool in tools} == set(TOOL_BUDGETS)
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+ "name",
+ [
+ "list_datasets",
+ "get_dataset_info",
+ "create_dataset",
+ "update_dataset",
+ "query_dataset",
+ ],
+)
+async def test_sql_dataset_inventory_routes_semantic_view_callers(name: str) -> None:
+ """Keep SQL scope in compact discovery and routing in tool descriptions."""
+ tool: Tool | None = await mcp.get_tool(name)
+ assert tool is not None
+ description: str = _create_search_result_serializer(MCP_TOOL_SEARCH_CONFIG)([tool])[
+ 0
+ ]["description"]
+ assert "SQL dataset" in description
+ assert tool.description is not None
+ assert "semantic view" in tool.description
+ assert "list_metrics" in tool.description
+ assert "get_table" in tool.description
+ # The list tool reserves most of its compact prose budget for mandatory
+ # request-wrapper and candidate-selection instructions.
+ if name != "list_datasets":
+ assert "semantic view" in description
+ assert "list_metrics" in description
+ assert "get_table" in description
+
+
@pytest.mark.asyncio
@pytest.mark.parametrize("name", TOOL_BUDGETS)
async def test_tool_inventory_size(name: str) -> None: