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/09/10 20:41:19 UTC
(superset) branch master updated: fix(mcp): make unknown dataset dimension errors actionable (#44106)
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 39944ffd470 fix(mcp): make unknown dataset dimension errors actionable (#44106)
39944ffd470 is described below
commit 39944ffd4703ebd3377ef7561228708f819d9e1d
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Thu Sep 10 16:41:12 2026 -0400
fix(mcp): make unknown dataset dimension errors actionable (#44106)
---
docs/admin_docs/configuration/mcp-server.mdx | 22 +++
superset/mcp_service/dataset/tool/query_dataset.py | 33 +++++
.../mcp_service/dataset/tool/test_query_dataset.py | 164 +++++++++++++++++++++
3 files changed, 219 insertions(+)
diff --git a/docs/admin_docs/configuration/mcp-server.mdx b/docs/admin_docs/configuration/mcp-server.mdx
index 292d909d314..c60a546f0b1 100644
--- a/docs/admin_docs/configuration/mcp-server.mdx
+++ b/docs/admin_docs/configuration/mcp-server.mdx
@@ -1038,3 +1038,25 @@ boundaries are not labeled UTC: dataset timezone or legacy hour-offset settings
can adjust the SQL comparisons. Use the applied filters and datasource settings
as well when explaining such queries. `query_dataset.applied_filters` retains the
original expressions so callers can compare the requested and resolved ranges.
+
+## Dataset query dimensions and nested fields
+
+The `query_dataset` tool accepts exact column names registered on the selected
+Superset dataset. Use `get_dataset_info` with the same `dataset_id` to discover
+those names; a column available on one dataset may not exist on another.
+
+A dotted name such as `address.city.name` can be used as a GROUP BY
+column when that exact name is registered. Registering only the parent
+`address` column does not register its nested fields. If a field is missing,
+refresh the dataset columns when the database exposes nested-field metadata,
+or add a calculated column with the warehouse-appropriate field-access
+expression and query its registered name. For ad-hoc expressions, use
+`execute_sql` instead.
+
+Unknown dimensions return the selected dataset's name and ID, up to ten
+available column names, and a pointer to `get_dataset_info` for the full list.
+Unregistered dotted dimensions also include shared nested-field setup guidance
+once, even when multiple dimensions are missing.
+The tool does not rewrite dots into SQL path separators: quoting and nested-field
+support depend on the dataset's database dialect, and dots can also be literal
+characters in column names.
diff --git a/superset/mcp_service/dataset/tool/query_dataset.py b/superset/mcp_service/dataset/tool/query_dataset.py
index 4adf08f1d44..f49dc158605 100644
--- a/superset/mcp_service/dataset/tool/query_dataset.py
+++ b/superset/mcp_service/dataset/tool/query_dataset.py
@@ -191,6 +191,39 @@ async def query_dataset( # noqa: C901
metrics_empty_hint=_NO_SAVED_METRICS_HINT,
)
+ missing_dimensions = [
+ name for name in request.columns if name not in valid_columns
+ ]
+ if missing_dimensions:
+ available = ", ".join(sorted(valid_columns)[:10]) or "(none)"
+ remaining = max(0, len(valid_columns) - 10)
+ if remaining:
+ available += f" (and {remaining} more)"
+ validation_errors.append(
+ f"Dataset '{dataset_name}' (id={dataset.id}). "
+ f"Available columns: {available}. "
+ "Use get_dataset_info with this dataset_id for the full column list."
+ )
+ dotted_missing = [name for name in missing_dimensions if "." in name]
+ if dotted_missing:
+ names = ", ".join(f"'{name}'" for name in dotted_missing)
+ registration = (
+ "is not registered as a column"
+ if len(dotted_missing) == 1
+ else "are not registered as columns"
+ )
+ validation_errors.append(
+ f"{names} {registration} on this dataset. "
+ "query_dataset requires exact registered column names; "
+ "registering a parent struct does not expose its "
+ "nested fields. "
+ "Refresh the dataset columns if the database exposes "
+ "this field, "
+ "or add a calculated column using the warehouse's field-access "
+ "expression and query its registered name. "
+ "Use execute_sql if you need an ad-hoc nested-field expression."
+ )
+
if validation_errors:
error_msg = "; ".join(validation_errors)
await ctx.error("Validation failed: %s" % (error_msg,))
diff --git a/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py b/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
index a4e95693fa2..fe8b667461e 100644
--- a/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
+++ b/tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py
@@ -1578,3 +1578,167 @@ async def test_query_dataset_cached_bounds_across_rollover(
assert cached_data["performance"]["cache_status"] == (
"no_data" if empty else "cached"
)
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize(
+ "dimensions",
+ [
+ ["address.city.name"],
+ ["event_name"],
+ ["address.city.name", "address.postal_code"],
+ ["address.city.name", "event_name", "address.postal_code"],
+ ],
+)
+async def test_query_dataset_unregistered_dimension_is_actionable(
+ mcp_server: FastMCP, dimensions: list[str]
+) -> None:
+ """Unregistered struct paths and wrong-dataset names explain how to recover."""
+ dataset = _make_dataset(
+ table_name="example_events",
+ columns=[_make_column("address"), _make_column("channel")],
+ )
+ with (
+ patch.object(query_dataset_module, "resolve_dataset", return_value=dataset),
+ patch.object(query_dataset_module, "execute_tabular_query") as execute,
+ ):
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "query_dataset",
+ {
+ "request": {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "columns": dimensions,
+ }
+ },
+ )
+ data = json.loads(result.content[0].text)
+ assert data["error_type"] == "ValidationError"
+ for dimension in dimensions:
+ assert dimension in data["error"]
+ assert "example_events" in data["error"]
+ assert "Available columns: address, channel" in data["error"]
+ assert "get_dataset_info" in data["error"]
+ dotted_dimensions = [name for name in dimensions if "." in name]
+ if dotted_dimensions:
+ assert "not registered" in data["error"]
+ for guidance in (
+ "query_dataset requires exact registered column names",
+ "registering a parent struct does not expose its nested fields",
+ "Refresh the dataset columns",
+ "add a calculated column",
+ "Use execute_sql",
+ ):
+ assert data["error"].count(guidance) == 1
+ else:
+ assert "nested fields" not in data["error"]
+ execute.assert_not_called()
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("engine", ["sqlite", "bigquery"])
+async def test_query_dataset_registered_dotted_groupby(
+ mcp_server: FastMCP, engine: str
+) -> None:
+ """Registered dotted names reach SQL generation without MCP sanitization.
+
+ SQLite treats the dot as part of a literal identifier, not a struct path.
+ Splitting every dotted dimension into SQL identifiers would break this case.
+ """
+ from sqlalchemy.dialects.sqlite import dialect
+
+ from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
+ from superset.models.core import Database
+
+ sql_dialect = (
+ pytest.importorskip("sqlalchemy_bigquery").BigQueryDialect()
+ if engine == "bigquery"
+ else dialect()
+ )
+ database = Database(
+ database_name="test",
+ sqlalchemy_uri="bigquery://test-project"
+ if engine == "bigquery"
+ else "sqlite://",
+ )
+ mock_engine = MagicMock()
+ mock_engine.dialect = sql_dialect
+ engine_context = MagicMock()
+ engine_context.__enter__.return_value = mock_engine
+ dimension = "address.city.name"
+ dataset = SqlaTable(
+ id=1,
+ table_name="example_locations",
+ database=database,
+ columns=[TableColumn(column_name=dimension, type="TEXT")],
+ metrics=[SqlMetric(metric_name="count", expression="COUNT(*)")],
+ )
+ compiled: list[str] = []
+
+ def compile_query(
+ dataset_id: int, datasource_type: str, query: dict[str, Any], **kwargs: Any
+ ) -> dict[str, Any]:
+ """Exercise the real dataset SQL builder without a warehouse connection."""
+ assert query["columns"] == [dimension]
+ sqla_query = dataset.get_sqla_query(
+ groupby=query["columns"], metrics=query["metrics"], is_timeseries=False
+ )
+ compiled.append(str(sqla_query.sqla_query.compile(dialect=sql_dialect)))
+ return _mock_command_result(
+ data=[{dimension: "Sports", "count": 2}], colnames=[dimension, "count"]
+ )
+
+ with (
+ patch.object(query_dataset_module, "resolve_dataset", return_value=dataset),
+ patch.object(database, "get_sqla_engine", return_value=engine_context),
+ patch.object(dataset, "get_sqla_row_level_filters", return_value=[]),
+ patch.object(
+ query_dataset_module, "execute_tabular_query", side_effect=compile_query
+ ),
+ ):
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "query_dataset",
+ {
+ "request": {
+ "dataset_id": 1,
+ "metrics": ["count"],
+ "columns": [dimension],
+ }
+ },
+ )
+ data = json.loads(result.content[0].text)
+ assert data["data"] == [{dimension: "Sports", "count": 2}]
+ assert len(compiled) == 1
+ if engine == "bigquery":
+ assert "`address`.`city`.`name`" in compiled[0]
+ assert "GROUP BY" in compiled[0]
+ else:
+ assert 'GROUP BY "address.city.name"' in compiled[0]
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("column_count", [0, 12])
+async def test_query_dataset_available_columns_preview(
+ mcp_server: FastMCP, column_count: int
+) -> None:
+ """Dimension errors bound the preview and handle datasets without columns."""
+ dataset = _make_dataset()
+ dataset.columns = [_make_column(f"col_{i:02}") for i in range(column_count)]
+ with patch.object(query_dataset_module, "resolve_dataset", return_value=dataset):
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "query_dataset",
+ {"request": {"dataset_id": 1, "columns": ["missing"]}},
+ )
+ data = json.loads(result.content[0].text)
+ assert data["error_type"] == "ValidationError"
+ if column_count:
+ assert "col_00" in data["error"]
+ assert "col_09" in data["error"]
+ assert "col_10" not in data["error"]
+ assert "(and 2 more)" in data["error"]
+ else:
+ assert "Available columns: (none)" in data["error"]
+ assert "get_dataset_info with this dataset_id" in data["error"]