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/04/20 21:50:20 UTC
(superset) branch master updated: feat(mcp): add get_chart_sql tool and expose chart filters in get_chart_info (#38700)
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 6948e73ec7b feat(mcp): add get_chart_sql tool and expose chart filters in get_chart_info (#38700)
6948e73ec7b is described below
commit 6948e73ec7b05d2485172b58fd5e902af2f384b3
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Mon Apr 20 17:50:10 2026 -0400
feat(mcp): add get_chart_sql tool and expose chart filters in get_chart_info (#38700)
---
superset/mcp_service/app.py | 2 +
superset/mcp_service/chart/schemas.py | 230 ++++++++
superset/mcp_service/chart/tool/__init__.py | 2 +
superset/mcp_service/chart/tool/get_chart_info.py | 5 +
superset/mcp_service/chart/tool/get_chart_sql.py | 512 ++++++++++++++++++
.../mcp_service/chart/tool/test_get_chart_sql.py | 592 +++++++++++++++++++++
6 files changed, 1343 insertions(+)
diff --git a/superset/mcp_service/app.py b/superset/mcp_service/app.py
index d2df64b7284..53df64a5c10 100644
--- a/superset/mcp_service/app.py
+++ b/superset/mcp_service/app.py
@@ -68,6 +68,7 @@ Chart Management:
- get_chart_info: Get detailed chart information by ID
- get_chart_preview: Get a visual preview of a chart with image URL
- get_chart_data: Get underlying chart data in text-friendly format
+- get_chart_sql: Get the rendered SQL query for a chart (without executing it)
- generate_chart: Create and save a new chart permanently
- generate_explore_link: Create an interactive explore URL (preferred for exploration)
- update_chart: Update existing saved chart configuration
@@ -479,6 +480,7 @@ from superset.mcp_service.chart.tool import ( # noqa: F401, E402
get_chart_data,
get_chart_info,
get_chart_preview,
+ get_chart_sql,
get_chart_type_schema,
list_charts,
update_chart,
diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py
index fae3faba721..e309d528d76 100644
--- a/superset/mcp_service/chart/schemas.py
+++ b/superset/mcp_service/chart/schemas.py
@@ -37,6 +37,7 @@ from pydantic import (
model_validator,
PositiveInt,
TypeAdapter,
+ ValidationError,
)
from typing_extensions import Self
@@ -126,6 +127,16 @@ class ChartInfo(BaseModel):
tags: List[TagInfo] = Field(default_factory=list, description="Chart tags")
owners: List[UserInfo] = Field(default_factory=list, description="Chart owners")
+ # Filters extracted from form_data for easy inspection
+ filters: ChartFiltersInfo | None = Field(
+ None,
+ description=(
+ "Structured representation of all filters applied to this chart, "
+ "extracted from form_data. Includes adhoc filters, time range, "
+ "extra filters, and custom WHERE/HAVING clauses."
+ ),
+ )
+
# Fields for unsaved state support
form_data: Dict[str, Any] | None = Field(
None,
@@ -289,6 +300,87 @@ def _humanize_timestamp(dt: datetime | None) -> str | None:
return humanize.naturaltime(datetime.now() - dt)
+def extract_filters_from_form_data(
+ form_data: Dict[str, Any] | None,
+) -> ChartFiltersInfo | None:
+ """Extract structured filter information from a chart's form_data.
+
+ Parses adhoc_filters, time_range, extra_filters, and custom
+ WHERE/HAVING clauses into a structured ChartFiltersInfo object.
+ Returns None if form_data is empty, not a dict, or has no filter-related fields.
+ """
+ if not form_data or not isinstance(form_data, dict):
+ return None
+
+ raw_adhoc = form_data.get("adhoc_filters", [])
+ adhoc_filters: List[AdhocFilter] = []
+ for f in raw_adhoc or []:
+ if not isinstance(f, dict):
+ continue
+ try:
+ adhoc_filters.append(
+ AdhocFilter(
+ clause=f.get("clause"),
+ expression_type=f.get("expressionType"),
+ subject=f.get("subject"),
+ operator=f.get("operator"),
+ comparator=f.get("comparator"),
+ sql_expression=f.get("sqlExpression"),
+ )
+ )
+ except (TypeError, ValueError, ValidationError):
+ # Skip malformed filter entries (e.g. non-string fields in
+ # corrupted cached state, legacy payloads, or Pydantic
+ # validation failures from unexpected field types)
+ continue
+
+ time_range = form_data.get("time_range")
+ granularity_sqla = form_data.get("granularity_sqla")
+ raw_extra = form_data.get("extra_filters", [])
+ extra_filters: List[Dict[str, Any]] = [
+ item
+ for item in (raw_extra if isinstance(raw_extra, list) else [])
+ if isinstance(item, dict)
+ and isinstance(item.get("col"), str)
+ and item.get("col")
+ ]
+ # Legacy top-level "filters" payload (older/simpler charts use this
+ # instead of adhoc_filters).
+ raw_filters = form_data.get("filters", [])
+ filters: List[Dict[str, Any]] = [
+ item
+ for item in (raw_filters if isinstance(raw_filters, list) else [])
+ if isinstance(item, dict)
+ and isinstance(item.get("col"), str)
+ and item.get("col")
+ ]
+ where = form_data.get("where")
+ having = form_data.get("having")
+
+ # Only return if there is at least one meaningful filter field
+ has_content = (
+ adhoc_filters
+ or time_range
+ or granularity_sqla
+ or extra_filters
+ or filters
+ or where
+ or having
+ )
+ if not has_content:
+ return None
+
+ return ChartFiltersInfo(
+ adhoc_filters=adhoc_filters,
+ time_range=time_range,
+ granularity_sqla=granularity_sqla,
+ extra_filters=extra_filters,
+ filters=filters,
+ where=where,
+ having=having,
+ )
+
+
def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
if not chart:
return None
@@ -312,6 +404,9 @@ def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
elif isinstance(chart_params, dict):
chart_form_data = chart_params
+ # Extract structured filter information
+ filters_info = extract_filters_from_form_data(chart_form_data)
+
return ChartInfo(
id=chart_id,
slice_name=getattr(chart, "slice_name", None),
@@ -324,6 +419,7 @@ def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
certification_details=getattr(chart, "certification_details", None),
cache_timeout=getattr(chart, "cache_timeout", None),
form_data=chart_form_data,
+ filters=filters_info,
changed_by=getattr(chart, "changed_by_name", None)
or (str(chart.changed_by) if getattr(chart, "changed_by", None) else None),
changed_by_name=getattr(chart, "changed_by_name", None),
@@ -1793,3 +1889,137 @@ class ChartPreview(BaseModel):
# Inherit versioning
schema_version: str = Field("2.0", description="Response schema version")
api_version: str = Field("v1", description="MCP API version")
+
+
+class GetChartSqlRequest(BaseModel):
+ """Request schema for get_chart_sql.
+
+ Returns the rendered SQL query that a chart would execute, without actually
+ running it. Useful for understanding what SQL a chart generates.
+
+ Provide a chart identifier (ID or UUID). Optionally provide form_data_key
+ to get the SQL for the unsaved chart state from the Explore view.
+ """
+
+ identifier: int | str | None = Field(
+ default=None,
+ description=(
+ "Chart identifier - can be numeric ID or UUID string. "
+ "Optional when form_data_key is provided (for unsaved charts)."
+ ),
+ )
+ form_data_key: str | None = Field(
+ default=None,
+ description=(
+ "Cache key for retrieving unsaved chart state. When a user "
+ "edits a chart in Explore but hasn't saved, the current state is stored "
+ "with this key. If provided, the tool returns the SQL for the unsaved "
+ "configuration instead of the saved version. "
+ "Can be used alone (without identifier) for unsaved charts."
+ ),
+ )
+
+ @model_validator(mode="after")
+ def validate_identifier_or_form_data_key(self) -> "GetChartSqlRequest":
+ if not self.identifier and not self.form_data_key:
+ raise ValueError(
+ "At least one of 'identifier' or 'form_data_key' must be provided."
+ )
+ return self
+
+
+class ChartSql(BaseModel):
+ """Response containing the rendered SQL for a chart."""
+
+ chart_id: int | None = Field(None, description="Chart ID (null for unsaved charts)")
+ chart_name: str | None = Field(
+ None, description="Chart name (null for unsaved charts)"
+ )
+ sql: str = Field(..., description="The rendered SQL query for this chart")
+ language: str | None = Field(None, description="Query language (e.g. 'sql')")
+ datasource_name: str | None = Field(
+ None, description="Name of the datasource the chart queries"
+ )
+ error: str | None = Field(
+ None,
+ description=(
+ "Validation or parse error if the SQL could not be fully generated. "
+ "When present, 'sql' may still contain a partial query."
+ ),
+ )
+
+
+class AdhocFilter(BaseModel):
+ """A single adhoc filter configured on a chart."""
+
+ clause: str | None = Field(None, description="SQL clause: WHERE or HAVING")
+ expression_type: str | None = Field(
+ None,
+ description=(
+ "Filter expression type: SIMPLE for column-based filters, "
+ "SQL for free-form SQL expressions"
+ ),
+ )
+ subject: str | None = Field(
+ None, description="Column name the filter applies to (SIMPLE filters)"
+ )
+ operator: str | None = Field(
+ None,
+ description="Filter operator (e.g. '==', '!=', 'IN', 'NOT IN', 'LIKE', '>')",
+ )
+ comparator: Any | None = Field(
+ None, description="Filter value(s) to compare against"
+ )
+ sql_expression: str | None = Field(
+ None,
+ description="Free-form SQL expression (SQL expression type filters only)",
+ )
+
+ model_config = ConfigDict(extra="ignore")
+
+
+class ChartFiltersInfo(BaseModel):
+ """Structured representation of all filters applied to a chart."""
+
+ adhoc_filters: List[AdhocFilter] = Field(
+ default_factory=list,
+ description=(
+ "Adhoc filters configured on the chart. These are the primary "
+ "filters visible in the chart's filter panel."
+ ),
+ )
+ time_range: str | None = Field(
+ None,
+ description=(
+ "Time range filter applied to the chart "
+ "(e.g. 'Last 7 days', 'No filter', '2024-01-01 : 2024-12-31')"
+ ),
+ )
+ granularity_sqla: str | None = Field(
+ None,
+ description="Temporal column used for time-based filtering",
+ )
+ extra_filters: List[Dict[str, Any]] = Field(
+ default_factory=list,
+ description=("Extra filters applied from dashboard context or URL parameters"),
+ )
+ filters: List[Dict[str, Any]] = Field(
+ default_factory=list,
+ description=(
+ "Legacy top-level filters payload (older/simpler charts). "
+ "These are plain column-operator-value dicts, distinct from "
+ "adhoc_filters which use the newer expressionType format."
+ ),
+ )
+ where: str | None = Field(
+ None,
+ description="Custom WHERE clause applied to the chart query",
+ )
+ having: str | None = Field(
+ None,
+ description="Custom HAVING clause applied to the chart query",
+ )
+
+
+# Rebuild ChartInfo so Pydantic can resolve the ChartFiltersInfo forward reference.
+ChartInfo.model_rebuild()
diff --git a/superset/mcp_service/chart/tool/__init__.py b/superset/mcp_service/chart/tool/__init__.py
index 0ad42852869..d13c62859e2 100644
--- a/superset/mcp_service/chart/tool/__init__.py
+++ b/superset/mcp_service/chart/tool/__init__.py
@@ -19,6 +19,7 @@ from .generate_chart import generate_chart
from .get_chart_data import get_chart_data
from .get_chart_info import get_chart_info
from .get_chart_preview import get_chart_preview
+from .get_chart_sql import get_chart_sql
from .get_chart_type_schema import get_chart_type_schema
from .list_charts import list_charts
from .update_chart import update_chart
@@ -27,6 +28,7 @@ from .update_chart_preview import update_chart_preview
__all__ = [
"list_charts",
"get_chart_info",
+ "get_chart_sql",
"generate_chart",
"update_chart",
"update_chart_preview",
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py b/superset/mcp_service/chart/tool/get_chart_info.py
index a550cf9d6fd..0477d28f039 100644
--- a/superset/mcp_service/chart/tool/get_chart_info.py
+++ b/superset/mcp_service/chart/tool/get_chart_info.py
@@ -32,6 +32,7 @@ from superset.mcp_service.chart.chart_utils import validate_chart_dataset
from superset.mcp_service.chart.schemas import (
ChartError,
ChartInfo,
+ extract_filters_from_form_data,
GetChartInfoRequest,
serialize_chart_object,
)
@@ -82,6 +83,7 @@ def _build_unsaved_chart_info(form_data_key: str) -> ChartInfo | ChartError:
viz_type=form_data.get("viz_type"),
datasource_name=form_data.get("datasource_name"),
datasource_type=form_data.get("datasource_type"),
+ filters=extract_filters_from_form_data(form_data),
form_data=form_data,
form_data_key=form_data_key,
is_unsaved_state=True,
@@ -101,6 +103,9 @@ def _apply_unsaved_state_override(result: ChartInfo, form_data_key: str) -> None
# Update viz_type from cached form_data if present
if result.form_data and "viz_type" in result.form_data:
result.viz_type = result.form_data["viz_type"]
+
+ # Update filters from cached form_data
+ result.filters = extract_filters_from_form_data(result.form_data)
except (TypeError, ValueError) as e:
logger.warning(
"Failed to parse cached form_data: %s. "
diff --git a/superset/mcp_service/chart/tool/get_chart_sql.py b/superset/mcp_service/chart/tool/get_chart_sql.py
new file mode 100644
index 00000000000..861d8ea89b2
--- /dev/null
+++ b/superset/mcp_service/chart/tool/get_chart_sql.py
@@ -0,0 +1,512 @@
+# 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.
+
+"""
+MCP tool: get_chart_sql
+"""
+
+import logging
+from typing import Any, TYPE_CHECKING
+
+from fastmcp import Context
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+if TYPE_CHECKING:
+ from superset.models.slice import Slice
+
+from superset.commands.exceptions import CommandException
+from superset.commands.explore.form_data.parameters import CommandParameters
+from superset.exceptions import SupersetException, SupersetSecurityException
+from superset.extensions import event_logger
+from superset.mcp_service.chart.chart_utils import validate_chart_dataset
+from superset.mcp_service.chart.schemas import (
+ ChartError,
+ ChartSql,
+ GetChartSqlRequest,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _get_cached_form_data(form_data_key: str) -> str | None:
+ """Retrieve form_data from cache using form_data_key.
+
+ Returns the JSON string of form_data if found, None otherwise.
+ """
+ from superset.commands.explore.form_data.get import GetFormDataCommand
+
+ try:
+ cmd_params = CommandParameters(key=form_data_key)
+ return GetFormDataCommand(cmd_params).run()
+ except (KeyError, ValueError, CommandException) as e:
+ logger.warning("Failed to retrieve form_data from cache: %s", e)
+ return None
+
+
+def _resolve_metrics(form_data: dict[str, Any], viz_type: str) -> list[Any]:
+ """Extract metrics from form_data, handling chart-type-specific fields."""
+ # Bubble charts store measures in x, y, size fields
+ if viz_type == "bubble":
+ return [m for field in ("x", "y", "size") if (m := form_data.get(field))]
+
+ metrics = form_data.get("metrics", [])
+ # Fallback: some chart types store the measure as singular "metric"
+ if not metrics and (metric := form_data.get("metric")):
+ metrics = [metric]
+ return metrics
+
+
+def _resolve_groupby(form_data: dict[str, Any]) -> list[str]:
+ """Extract groupby columns from form_data with fallback aliases.
+
+ Normalises scalar strings (e.g. heatmap_v2 migrated from legacy
+ ``all_columns_y``) so that ``list("country")`` does not split into
+ individual characters.
+ """
+ raw_groupby = form_data.get("groupby") or []
+ if isinstance(raw_groupby, str):
+ groupby: list[str] = [raw_groupby]
+ else:
+ groupby = list(raw_groupby)
+
+ if groupby:
+ return groupby
+
+ # Fallback: some chart types store dimensions in entity/series/columns
+ for field in ("entity", "series"):
+ value = form_data.get(field)
+ if isinstance(value, str) and value not in groupby:
+ groupby.append(value)
+
+ form_columns = form_data.get("columns")
+ if isinstance(form_columns, list):
+ for col in form_columns:
+ if isinstance(col, str) and col not in groupby:
+ groupby.append(col)
+
+ return groupby
+
+
+def _resolve_metrics_and_groupby(
+ form_data: dict[str, Any],
+ chart: "Slice | None",
+) -> tuple[list[Any], list[str]]:
+ """Resolve metrics and groupby columns from form_data.
+
+ Handles chart-type-specific field names: singular ``metric`` for
+ big-number variants, bubble ``x``/``y``/``size``, and fallback
+ fields ``entity``, ``series``, and ``columns`` for dimensions.
+ """
+ viz_type = form_data.get(
+ "viz_type", getattr(chart, "viz_type", "") if chart else ""
+ )
+
+ singular_metric_no_groupby = (
+ "big_number",
+ "big_number_total",
+ "pop_kpi",
+ )
+ if viz_type in singular_metric_no_groupby:
+ metrics: list[Any] = [metric] if (metric := form_data.get("metric")) else []
+ return metrics, []
+
+ return _resolve_metrics(form_data, viz_type), _resolve_groupby(form_data)
+
+
+def _build_query_context_from_form_data(
+ form_data: dict[str, Any],
+ chart: "Slice | None" = None,
+) -> Any:
+ """Build a QueryContext from form_data with result_type=QUERY.
+
+ This constructs a query context that will return the rendered SQL
+ instead of executing the query.
+ """
+ from superset.common.chart_data import ChartDataResultType
+ from superset.common.query_context_factory import QueryContextFactory
+
+ factory = QueryContextFactory()
+
+ datasource_id = form_data.get("datasource_id")
+ datasource_type = form_data.get("datasource_type")
+
+ # Unsaved Explore state often stores datasource as a combined field
+ # like "123__table" instead of separate datasource_id/datasource_type.
+ if not datasource_id and (combined := form_data.get("datasource")):
+ if isinstance(combined, str) and "__" in combined:
+ parts = combined.split("__", 1)
+ datasource_id = int(parts[0]) if parts[0].isdigit() else parts[0]
+ datasource_type = parts[1] if len(parts) > 1 else None
+
+ if not datasource_id and chart:
+ datasource_id = getattr(chart, "datasource_id", None)
+ if not datasource_type and chart:
+ datasource_type = getattr(chart, "datasource_type", None)
+
+ metrics, groupby = _resolve_metrics_and_groupby(form_data, chart)
+
+ # Build a minimal query object; let QueryContextFactory handle temporal
+ # fields (time_range, granularity_sqla), adhoc_filters, WHERE/HAVING
+ # clauses, etc. from form_data — same approach as get_chart_data.
+ query_dict: dict[str, Any] = {
+ "columns": groupby,
+ "metrics": metrics,
+ }
+
+ if (row_limit := form_data.get("row_limit")) is not None:
+ query_dict["row_limit"] = row_limit
+
+ # Ensure datasource fields satisfy DatasourceDict typing requirements.
+ # datasource_id must be int | str; datasource_type must be str.
+ if not isinstance(datasource_id, (int, str)):
+ raise ValueError(
+ "Cannot determine datasource ID from form_data. "
+ "Provide a chart identifier or ensure form_data contains "
+ "'datasource_id' or 'datasource'."
+ )
+ resolved_id: int | str = datasource_id
+ resolved_type: str = (
+ datasource_type if isinstance(datasource_type, str) else "table"
+ )
+
+ return factory.create(
+ datasource={"id": resolved_id, "type": resolved_type},
+ queries=[query_dict],
+ form_data=form_data,
+ result_type=ChartDataResultType.QUERY,
+ force=False,
+ )
+
+
+def _find_chart_by_identifier(
+ identifier: int | str,
+) -> "Slice | None":
+ """Look up a chart by numeric ID or UUID string."""
+ from superset.daos.chart import ChartDAO
+
+ if isinstance(identifier, int) or (
+ isinstance(identifier, str) and identifier.isdigit()
+ ):
+ chart_id = int(identifier) if isinstance(identifier, str) else identifier
+ return ChartDAO.find_by_id(chart_id)
+ if isinstance(identifier, str):
+ return ChartDAO.find_by_id(identifier, id_column="uuid")
+ return None
+
+
+def _resolve_effective_form_data(
+ chart: "Slice",
+ form_data_key: str | None,
+) -> tuple[dict[str, Any], bool]:
+ """Resolve the effective form_data for a chart.
+
+ Returns (form_data_dict, using_unsaved_state).
+ """
+ from superset.utils import json as utils_json
+
+ if form_data_key:
+ if cached_raw := _get_cached_form_data(form_data_key):
+ try:
+ parsed = utils_json.loads(cached_raw)
+ if isinstance(parsed, dict):
+ return parsed, True
+ except (TypeError, ValueError):
+ pass
+
+ try:
+ saved = utils_json.loads(chart.params) if chart.params else {}
+ except (TypeError, ValueError):
+ saved = {}
+ return saved if isinstance(saved, dict) else {}, False
+
+
+def _sql_from_saved_query_context(
+ chart: "Slice",
+) -> ChartSql | ChartError | None:
+ """Try to extract SQL from a chart's saved query_context.
+
+ Returns None if the chart has no query_context or parsing fails.
+ """
+ from superset.charts.schemas import ChartDataQueryContextSchema
+ from superset.commands.chart.data.get_data_command import ChartDataCommand
+ from superset.common.chart_data import ChartDataResultType
+ from superset.utils import json as utils_json
+
+ if not chart.query_context:
+ return None
+
+ try:
+ qc_json = utils_json.loads(chart.query_context)
+ qc_json["result_type"] = ChartDataResultType.QUERY
+ qc_json["force"] = False
+
+ query_context = ChartDataQueryContextSchema().load(qc_json)
+ query_context.result_type = ChartDataResultType.QUERY
+
+ command = ChartDataCommand(query_context)
+ command.validate()
+ result = command.run()
+
+ return _extract_sql_from_result(
+ result, chart.id, chart.slice_name, chart.datasource_name
+ )
+ except SupersetSecurityException:
+ raise # Let access denials propagate for consistent error handling
+ except (SupersetException, CommandException, TypeError, ValueError):
+ return None
+
+
+def _sql_from_form_data(
+ form_data: dict[str, Any],
+ chart: "Slice | None",
+) -> ChartSql | ChartError:
+ """Build SQL from form_data (fallback path)."""
+ from superset.commands.chart.data.get_data_command import ChartDataCommand
+
+ query_context = _build_query_context_from_form_data(form_data, chart)
+ command = ChartDataCommand(query_context)
+ command.validate()
+ result = command.run()
+
+ return _extract_sql_from_result(
+ result,
+ chart_id=getattr(chart, "id", None),
+ chart_name=getattr(chart, "slice_name", None),
+ datasource_name=getattr(chart, "datasource_name", None),
+ )
+
+
+def _extract_sql_from_result(
+ result: dict[str, Any],
+ chart_id: int | None,
+ chart_name: str | None,
+ datasource_name: str | None,
+) -> ChartSql | ChartError:
+ """Extract SQL query string(s) from the ChartDataCommand result.
+
+ Charts with multiple queries (e.g. mixed timeseries) produce multiple
+ query entries. This collects SQL from all of them so that composite
+ visualisations do not silently lose part of their SQL.
+ """
+ queries = result.get("queries", [])
+ if not queries:
+ return ChartError(
+ error=(
+ "No query results returned. The chart may have an empty configuration."
+ ),
+ error_type="EmptyQuery",
+ )
+
+ sql_parts: list[str] = []
+ errors: list[str] = []
+ language = "sql"
+
+ for idx, query_result in enumerate(queries, start=1):
+ if not isinstance(query_result, dict):
+ continue
+ if "language" in query_result and isinstance(query_result["language"], str):
+ language = query_result["language"]
+ query_sql = query_result.get("query", "")
+ query_error = query_result.get("error")
+
+ if query_sql:
+ sql_parts.append(
+ query_sql if len(queries) == 1 else f"-- Query {idx}\n{query_sql}"
+ )
+ if query_error:
+ errors.append(f"Query {idx}: {query_error}")
+
+ if not sql_parts and errors:
+ return ChartError(
+ error="SQL generation failed: " + "; ".join(errors),
+ error_type="QueryGenerationFailed",
+ )
+
+ return ChartSql(
+ chart_id=chart_id,
+ chart_name=chart_name,
+ sql="\n\n".join(sql_parts),
+ language=language,
+ datasource_name=datasource_name,
+ error="; ".join(errors) if errors else None,
+ )
+
+
+@tool(
+ tags=["data"],
+ class_permission_name="Chart",
+ annotations=ToolAnnotations(
+ title="Get chart SQL",
+ readOnlyHint=True,
+ destructiveHint=False,
+ ),
+)
+async def get_chart_sql(
+ request: GetChartSqlRequest, ctx: Context
+) -> ChartSql | ChartError:
+ """Get the rendered SQL query for a chart without executing it.
+
+ Returns the SQL that a chart would execute, similar to the "View Query"
+ feature in the Superset UI. Useful for understanding, debugging, or
+ auditing the SQL generated by a chart's configuration.
+
+ Note: The returned SQL is RLS-scoped to the caller and contains
+ table/schema names. Same disclosure surface as the Superset UI
+ "View Query" modal. Consumers should not log or share the output
+ without considering data-access implications.
+
+ Supports:
+ - Numeric ID or UUID lookup
+ - form_data_key: get SQL for unsaved chart state from Explore view
+
+ Example usage:
+ ```json
+ {
+ "identifier": 123
+ }
+ ```
+
+ Returns the SQL query string and metadata about the chart.
+ """
+ await ctx.info(
+ "Starting chart SQL retrieval: identifier=%s, form_data_key=%s"
+ % (request.identifier, request.form_data_key)
+ )
+
+ try:
+ return await _handle_chart_sql_request(request, ctx)
+ except SupersetException as e:
+ logger.exception("Superset error in get_chart_sql")
+ return ChartError(
+ error=f"Superset error: {e}",
+ error_type="SupersetError",
+ )
+ except (ValueError, TypeError) as e:
+ logger.exception("Unexpected error in get_chart_sql")
+ return ChartError(
+ error=f"Failed to generate chart SQL: {e}",
+ error_type="QueryGenerationFailed",
+ )
+
+
+async def _handle_chart_sql_request(
+ request: GetChartSqlRequest, ctx: Context
+) -> ChartSql | ChartError:
+ """Core logic for get_chart_sql, extracted for complexity."""
+ # Handle unsaved chart (form_data_key only, no identifier)
+ if not request.identifier and request.form_data_key:
+ return await _handle_unsaved_chart_sql(request.form_data_key, ctx)
+
+ # Find the chart by identifier
+ if request.identifier is None:
+ return ChartError(
+ error="Chart identifier is required.",
+ error_type="ValidationError",
+ )
+ with event_logger.log_context(action="mcp.get_chart_sql.chart_lookup"):
+ chart = _find_chart_by_identifier(request.identifier)
+
+ if not chart:
+ return ChartError(
+ error=f"No chart found with identifier: {request.identifier}",
+ error_type="NotFound",
+ )
+
+ await ctx.info(
+ "Chart found: chart_id=%s, chart_name=%s, viz_type=%s"
+ % (chart.id, chart.slice_name, chart.viz_type)
+ )
+
+ # Validate the chart's dataset is accessible
+ validation_result = validate_chart_dataset(chart, check_access=True)
+ if not validation_result.is_valid:
+ await ctx.warning(
+ "Chart found but dataset is not accessible: %s" % (validation_result.error,)
+ )
+ return ChartError(
+ error=validation_result.error or "Chart's dataset is not accessible.",
+ error_type="DatasetNotAccessible",
+ )
+ for warning in validation_result.warnings:
+ await ctx.warning("Dataset warning: %s" % (warning,))
+
+ # Resolve effective form_data
+ effective_form_data, using_unsaved_state = _resolve_effective_form_data(
+ chart, request.form_data_key
+ )
+
+ # Try saved query_context first (faster, more accurate)
+ with event_logger.log_context(action="mcp.get_chart_sql.build_query"):
+ if not using_unsaved_state:
+ saved_result = _sql_from_saved_query_context(chart)
+ if saved_result is not None:
+ return saved_result
+ await ctx.warning(
+ "Could not extract SQL from saved query_context. "
+ "Falling back to form_data."
+ )
+
+ # Fallback: build query context from form_data
+ try:
+ return _sql_from_form_data(effective_form_data, chart)
+ except (SupersetException, CommandException, ValueError) as e:
+ await ctx.warning("Failed to build SQL from form_data: %s" % str(e))
+ return ChartError(
+ error="Failed to generate SQL for chart %s: %s" % (chart.id, e),
+ error_type="QueryGenerationFailed",
+ )
+
+
+async def _handle_unsaved_chart_sql(
+ form_data_key: str, ctx: Context
+) -> ChartSql | ChartError:
+ """Handle SQL retrieval for unsaved charts (form_data_key only)."""
+ from superset.utils import json as utils_json
+
+ with event_logger.log_context(action="mcp.get_chart_sql.unsaved_chart_from_cache"):
+ await ctx.info(
+ "No chart identifier - getting SQL from unsaved chart cache: "
+ "form_data_key=%s" % (form_data_key,)
+ )
+ cached_form_data = _get_cached_form_data(form_data_key)
+ if not cached_form_data:
+ return ChartError(
+ error="No cached chart data found for form_data_key. "
+ "The cache may have expired.",
+ error_type="NotFound",
+ )
+ try:
+ form_data = utils_json.loads(cached_form_data)
+ except (TypeError, ValueError) as e:
+ return ChartError(
+ error=f"Failed to parse cached form_data: {e}",
+ error_type="ParseError",
+ )
+ if not isinstance(form_data, dict):
+ return ChartError(
+ error="Cached form_data is not a valid JSON object.",
+ error_type="ParseError",
+ )
+
+ try:
+ return _sql_from_form_data(form_data, chart=None)
+ except (SupersetException, CommandException, ValueError) as e:
+ await ctx.warning("Failed to generate SQL from form_data: %s" % str(e))
+ return ChartError(
+ error="Failed to generate SQL from cached form_data: %s" % str(e),
+ error_type="QueryGenerationFailed",
+ )
diff --git a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py
new file mode 100644
index 00000000000..00fa65991c5
--- /dev/null
+++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_sql.py
@@ -0,0 +1,592 @@
+# 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.
+
+"""
+Unit tests for get_chart_sql MCP tool
+"""
+
+import importlib
+from unittest.mock import Mock, patch
+
+import pytest
+
+from superset.mcp_service.chart.schemas import (
+ ChartError,
+ ChartSql,
+ GetChartSqlRequest,
+)
+from superset.mcp_service.chart.tool.get_chart_sql import (
+ _build_query_context_from_form_data,
+ _extract_sql_from_result,
+ _find_chart_by_identifier,
+ _resolve_effective_form_data,
+ _resolve_groupby,
+ _resolve_metrics,
+ _resolve_metrics_and_groupby,
+)
+
+_get_chart_sql_mod = importlib.import_module(
+ "superset.mcp_service.chart.tool.get_chart_sql"
+)
+
+
+class TestGetChartSqlRequestSchema:
+ """Tests for GetChartSqlRequest schema validation."""
+
+ def test_valid_request_with_identifier(self):
+ """Test creating request with a numeric identifier."""
+ request = GetChartSqlRequest(identifier=123)
+ assert request.identifier == 123
+ assert request.form_data_key is None
+
+ def test_valid_request_with_uuid_identifier(self):
+ """Test creating request with a UUID string identifier."""
+ request = GetChartSqlRequest(identifier="abc-def-123")
+ assert request.identifier == "abc-def-123"
+ assert request.form_data_key is None
+
+ def test_valid_request_with_form_data_key_only(self):
+ """Test creating request with form_data_key only (unsaved chart)."""
+ request = GetChartSqlRequest(form_data_key="some-key")
+ assert request.identifier is None
+ assert request.form_data_key == "some-key"
+
+ def test_valid_request_with_both(self):
+ """Test creating request with both identifier and form_data_key."""
+ request = GetChartSqlRequest(identifier=42, form_data_key="some-key")
+ assert request.identifier == 42
+ assert request.form_data_key == "some-key"
+
+ def test_invalid_request_neither_provided(self):
+ """Test request fails when neither identifier nor key given."""
+ with pytest.raises(ValueError, match="At least one of"):
+ GetChartSqlRequest()
+
+
+class TestExtractSqlFromResult:
+ """Tests for the _extract_sql_from_result helper."""
+
+ def test_successful_sql_extraction(self):
+ """Test extracting SQL from a normal result."""
+ result = {
+ "queries": [
+ {
+ "query": "SELECT * FROM my_table WHERE x > 1",
+ "language": "sql",
+ }
+ ]
+ }
+ output = _extract_sql_from_result(
+ result,
+ chart_id=10,
+ chart_name="Sales Chart",
+ datasource_name="my_table",
+ )
+ assert isinstance(output, ChartSql)
+ assert output.sql == "SELECT * FROM my_table WHERE x > 1"
+ assert output.language == "sql"
+ assert output.chart_id == 10
+ assert output.chart_name == "Sales Chart"
+ assert output.datasource_name == "my_table"
+ assert output.error is None
+
+ def test_empty_queries_returns_error(self):
+ """Test that empty query results return a ChartError."""
+ result = {"queries": []}
+ output = _extract_sql_from_result(
+ result, chart_id=1, chart_name="Test", datasource_name="ds"
+ )
+ assert isinstance(output, ChartError)
+ assert output.error_type == "EmptyQuery"
+
+ def test_missing_queries_key_returns_error(self):
+ """Test that missing 'queries' key returns a ChartError."""
+ result = {}
+ output = _extract_sql_from_result(
+ result, chart_id=1, chart_name="Test", datasource_name="ds"
+ )
+ assert isinstance(output, ChartError)
+ assert output.error_type == "EmptyQuery"
+
+ def test_no_sql_with_error_returns_chart_error(self):
+ """Test that empty sql with an error message returns ChartError."""
+ result = {
+ "queries": [
+ {
+ "query": "",
+ "language": "sql",
+ "error": "Unknown column 'foo'",
+ }
+ ]
+ }
+ output = _extract_sql_from_result(
+ result, chart_id=5, chart_name="Bad Chart", datasource_name="ds"
+ )
+ assert isinstance(output, ChartError)
+ assert output.error_type == "QueryGenerationFailed"
+ assert "Unknown column" in output.error
+
+ def test_sql_with_error_returns_chart_sql(self):
+ """Test that partial SQL with a non-fatal error returns ChartSql with error."""
+ result = {
+ "queries": [
+ {
+ "query": "SELECT col1 FROM tbl",
+ "language": "sql",
+ "error": "Warning: column col2 not found",
+ }
+ ]
+ }
+ output = _extract_sql_from_result(
+ result, chart_id=7, chart_name="Partial", datasource_name="tbl"
+ )
+ assert isinstance(output, ChartSql)
+ assert output.sql == "SELECT col1 FROM tbl"
+ assert output.error is not None
+
+ def test_null_chart_metadata(self):
+ """Test extraction when chart metadata is None."""
+ result = {"queries": [{"query": "SELECT 1", "language": "sql"}]}
+ output = _extract_sql_from_result(
+ result, chart_id=None, chart_name=None, datasource_name=None
+ )
+ assert isinstance(output, ChartSql)
+ assert output.chart_id is None
+ assert output.chart_name is None
+ assert output.datasource_name is None
+
+
+class TestFindChartByIdentifier:
+ """Tests for the _find_chart_by_identifier helper."""
+
+ @patch("superset.daos.chart.ChartDAO.find_by_id")
+ def test_find_by_numeric_id(self, mock_find):
+ """Test finding chart by numeric ID."""
+ mock_chart = Mock()
+ mock_find.return_value = mock_chart
+
+ result = _find_chart_by_identifier(42)
+
+ mock_find.assert_called_once_with(42)
+ assert result == mock_chart
+
+ @patch("superset.daos.chart.ChartDAO.find_by_id")
+ def test_find_by_digit_string(self, mock_find):
+ """Test finding chart by digit string (treated as numeric ID)."""
+ mock_chart = Mock()
+ mock_find.return_value = mock_chart
+
+ result = _find_chart_by_identifier("123")
+
+ mock_find.assert_called_once_with(123)
+ assert result == mock_chart
+
+ @patch("superset.daos.chart.ChartDAO.find_by_id")
+ def test_find_by_uuid_string(self, mock_find):
+ """Test finding chart by UUID string."""
+ mock_chart = Mock()
+ mock_find.return_value = mock_chart
+
+ result = _find_chart_by_identifier("abc-123-def")
+
+ mock_find.assert_called_once_with("abc-123-def", id_column="uuid")
+ assert result == mock_chart
+
+ @patch("superset.daos.chart.ChartDAO.find_by_id")
+ def test_not_found_returns_none(self, mock_find):
+ """Test that None is returned when chart is not found."""
+ mock_find.return_value = None
+
+ result = _find_chart_by_identifier(999)
+
+ assert result is None
+
+
+class TestResolveEffectiveFormData:
+ """Tests for the _resolve_effective_form_data helper."""
+
+ def test_returns_saved_params_when_no_form_data_key(self):
+ """Test that saved chart params are returned when no form_data_key."""
+ mock_chart = Mock()
+ mock_chart.params = '{"metrics": ["count"], "viz_type": "table"}'
+
+ form_data, using_unsaved = _resolve_effective_form_data(
+ mock_chart, form_data_key=None
+ )
+
+ assert form_data == {"metrics": ["count"], "viz_type": "table"}
+ assert using_unsaved is False
+
+ def test_returns_empty_dict_when_no_params(self):
+ """Test that empty dict is returned when chart has no params."""
+ mock_chart = Mock()
+ mock_chart.params = None
+
+ form_data, using_unsaved = _resolve_effective_form_data(
+ mock_chart, form_data_key=None
+ )
+
+ assert form_data == {}
+ assert using_unsaved is False
+
+ def test_returns_empty_dict_for_invalid_json_params(self):
+ """Test that invalid JSON in params returns empty dict."""
+ mock_chart = Mock()
+ mock_chart.params = "not-valid-json"
+
+ form_data, using_unsaved = _resolve_effective_form_data(
+ mock_chart, form_data_key=None
+ )
+
+ assert form_data == {}
+ assert using_unsaved is False
+
+ @patch.object(_get_chart_sql_mod, "_get_cached_form_data")
+ def test_returns_cached_form_data_when_key_provided(self, mock_get_cached):
+ """Test that cached form_data is used when form_data_key is provided."""
+ mock_get_cached.return_value = '{"metrics": ["sum_sales"], "unsaved": true}'
+ mock_chart = Mock()
+ mock_chart.params = '{"metrics": ["count"]}'
+
+ form_data, using_unsaved = _resolve_effective_form_data(
+ mock_chart, form_data_key="test-key-123"
+ )
+
+ assert form_data == {"metrics": ["sum_sales"], "unsaved": True}
+ assert using_unsaved is True
+
+ @patch.object(_get_chart_sql_mod, "_get_cached_form_data")
+ def test_falls_back_to_saved_when_cache_miss(self, mock_get_cached):
+ """Test fallback to saved params when cache returns nothing."""
+ mock_get_cached.return_value = None
+ mock_chart = Mock()
+ mock_chart.params = '{"metrics": ["count"]}'
+
+ form_data, using_unsaved = _resolve_effective_form_data(
+ mock_chart, form_data_key="expired-key"
+ )
+
+ assert form_data == {"metrics": ["count"]}
+ assert using_unsaved is False
+
+
+class TestResolveMetricsAndGroupby:
+ """Tests for metric/groupby resolution helpers."""
+
+ def test_bubble_chart_extracts_x_y_size(self):
+ """Bubble charts store measures in x, y, size fields."""
+ form_data = {
+ "viz_type": "bubble",
+ "x": "col_x",
+ "y": "col_y",
+ "size": "col_size",
+ }
+ metrics = _resolve_metrics(form_data, "bubble")
+ assert metrics == ["col_x", "col_y", "col_size"]
+
+ def test_bubble_chart_via_resolve_metrics_and_groupby(self):
+ """Integration: _resolve_metrics_and_groupby handles bubble type."""
+ form_data = {
+ "viz_type": "bubble",
+ "x": "metric_a",
+ "y": "metric_b",
+ "size": "metric_c",
+ "groupby": ["dim1"],
+ }
+ metrics, groupby = _resolve_metrics_and_groupby(form_data, chart=None)
+ assert metrics == ["metric_a", "metric_b", "metric_c"]
+ assert groupby == ["dim1"]
+
+ def test_bubble_chart_missing_fields(self):
+ """Bubble charts with missing x/y/size fields produce partial metrics."""
+ form_data = {"viz_type": "bubble", "x": "only_x"}
+ metrics = _resolve_metrics(form_data, "bubble")
+ assert metrics == ["only_x"]
+
+ def test_string_groupby_normalised_to_list(self):
+ """A scalar string groupby must become a single-item list, not chars."""
+ groupby = _resolve_groupby({"groupby": "country"})
+ assert groupby == ["country"]
+
+ def test_list_groupby_unchanged(self):
+ """A list groupby should pass through normally."""
+ groupby = _resolve_groupby({"groupby": ["col_a", "col_b"]})
+ assert groupby == ["col_a", "col_b"]
+
+ def test_empty_groupby_falls_back_to_entity_series(self):
+ """Missing groupby falls back to entity/series/columns."""
+ form_data = {"entity": "country", "series": "year"}
+ groupby = _resolve_groupby(form_data)
+ assert groupby == ["country", "year"]
+
+ def test_empty_groupby_falls_back_to_columns(self):
+ """Missing groupby falls back to columns list."""
+ form_data = {"columns": ["a", "b"]}
+ groupby = _resolve_groupby(form_data)
+ assert groupby == ["a", "b"]
+
+
+class TestBuildQueryContextFromFormData:
+ """Regression tests for fallback query context construction.
+
+ Verifies that temporal fields, adhoc_filters, and legacy filters from
+ form_data are passed through to QueryContextFactory (not dropped).
+ """
+
+ @patch("superset.common.query_context_factory.QueryContextFactory")
+ def test_temporal_fields_passed_to_factory(self, mock_factory_cls):
+ """time_range, granularity_sqla, adhoc_filters from form_data are
+ forwarded via form_data= to the factory, not dropped."""
+ mock_factory = Mock()
+ mock_factory.create.return_value = Mock()
+ mock_factory_cls.return_value = mock_factory
+
+ form_data = {
+ "datasource_id": 1,
+ "datasource_type": "table",
+ "metrics": ["count"],
+ "groupby": ["country"],
+ "time_range": "Last 7 days",
+ "granularity_sqla": "created_at",
+ "adhoc_filters": [
+ {
+ "clause": "WHERE",
+ "expressionType": "SIMPLE",
+ "subject": "status",
+ "operator": "==",
+ "comparator": "active",
+ }
+ ],
+ "where": "region = 'US'",
+ "having": "count > 10",
+ "filters": [{"col": "city", "op": "==", "val": "NYC"}],
+ }
+
+ with patch(
+ "superset.common.chart_data.ChartDataResultType"
+ ) as mock_result_type:
+ mock_result_type.QUERY = "QUERY"
+ _build_query_context_from_form_data(form_data, chart=None)
+
+ # Verify factory.create was called with form_data containing all fields
+ call_kwargs = mock_factory.create.call_args[1]
+ assert call_kwargs["form_data"] is form_data
+ assert call_kwargs["form_data"]["time_range"] == "Last 7 days"
+ assert call_kwargs["form_data"]["granularity_sqla"] == "created_at"
+ assert call_kwargs["form_data"]["adhoc_filters"] is not None
+ assert call_kwargs["form_data"]["where"] == "region = 'US'"
+ assert call_kwargs["form_data"]["having"] == "count > 10"
+ assert call_kwargs["form_data"]["filters"] == [
+ {"col": "city", "op": "==", "val": "NYC"}
+ ]
+
+ @patch("superset.common.query_context_factory.QueryContextFactory")
+ def test_metrics_and_groupby_in_queries(self, mock_factory_cls):
+ """Resolved metrics and groupby are passed in queries parameter."""
+ mock_factory = Mock()
+ mock_factory.create.return_value = Mock()
+ mock_factory_cls.return_value = mock_factory
+
+ form_data = {
+ "datasource_id": 1,
+ "datasource_type": "table",
+ "metrics": ["sum_revenue"],
+ "groupby": ["product"],
+ }
+
+ with patch(
+ "superset.common.chart_data.ChartDataResultType"
+ ) as mock_result_type:
+ mock_result_type.QUERY = "QUERY"
+ _build_query_context_from_form_data(form_data, chart=None)
+
+ call_kwargs = mock_factory.create.call_args[1]
+ queries = call_kwargs["queries"]
+ assert len(queries) == 1
+ assert queries[0]["metrics"] == ["sum_revenue"]
+ assert queries[0]["columns"] == ["product"]
+
+
+class TestGetChartSqlTool:
+ """Integration-style tests for the get_chart_sql MCP tool via Client."""
+
+ @pytest.fixture(autouse=True)
+ def mock_auth(self):
+ """Mock authentication for all tests."""
+ with patch("superset.mcp_service.auth.get_user_from_request") as mock_get_user:
+ mock_user = Mock()
+ mock_user.id = 1
+ mock_user.username = "admin"
+ mock_get_user.return_value = mock_user
+ yield mock_get_user
+
+ @pytest.fixture
+ def mcp_server(self):
+ from superset.mcp_service.app import mcp
+
+ return mcp
+
+ @patch.object(_get_chart_sql_mod, "validate_chart_dataset")
+ @patch.object(_get_chart_sql_mod, "_find_chart_by_identifier")
+ @pytest.mark.asyncio
+ async def test_chart_not_found(self, mock_find, mock_validate, mcp_server):
+ """Test that a not-found chart returns an error."""
+ from fastmcp import Client
+
+ mock_find.return_value = None
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "get_chart_sql", {"request": {"identifier": 999}}
+ )
+
+ data = result.structured_content.get("result", result.structured_content)
+ assert data["error_type"] == "NotFound"
+ assert "999" in data["error"]
+
+ @patch.object(_get_chart_sql_mod, "_sql_from_form_data")
+ @patch.object(_get_chart_sql_mod, "_sql_from_saved_query_context")
+ @patch.object(_get_chart_sql_mod, "_resolve_effective_form_data")
+ @patch.object(_get_chart_sql_mod, "validate_chart_dataset")
+ @patch.object(_get_chart_sql_mod, "_find_chart_by_identifier")
+ @pytest.mark.asyncio
+ async def test_success_via_saved_query_context(
+ self,
+ mock_find,
+ mock_validate,
+ mock_resolve,
+ mock_saved_qc,
+ mock_form_data_sql,
+ mcp_server,
+ ):
+ """Test successful SQL retrieval via saved query_context."""
+ from fastmcp import Client
+
+ from superset.mcp_service.chart.chart_utils import (
+ DatasetValidationResult,
+ )
+
+ mock_chart = Mock()
+ mock_chart.id = 10
+ mock_chart.slice_name = "Sales Chart"
+ mock_chart.viz_type = "table"
+ mock_find.return_value = mock_chart
+
+ mock_validate.return_value = DatasetValidationResult(
+ is_valid=True, dataset_id=1, dataset_name="ds", warnings=[]
+ )
+ mock_resolve.return_value = ({"metrics": ["count"]}, False)
+ mock_saved_qc.return_value = ChartSql(
+ chart_id=10,
+ chart_name="Sales Chart",
+ sql="SELECT COUNT(*) FROM sales",
+ language="sql",
+ datasource_name="sales",
+ )
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "get_chart_sql", {"request": {"identifier": 10}}
+ )
+
+ data = result.structured_content.get("result", result.structured_content)
+ assert "SELECT COUNT(*) FROM sales" in data["sql"]
+ assert data["chart_id"] == 10
+
+ @patch.object(_get_chart_sql_mod, "_sql_from_form_data")
+ @patch.object(_get_chart_sql_mod, "_sql_from_saved_query_context")
+ @patch.object(_get_chart_sql_mod, "_resolve_effective_form_data")
+ @patch.object(_get_chart_sql_mod, "validate_chart_dataset")
+ @patch.object(_get_chart_sql_mod, "_find_chart_by_identifier")
+ @pytest.mark.asyncio
+ async def test_fallback_to_form_data_when_saved_qc_fails(
+ self,
+ mock_find,
+ mock_validate,
+ mock_resolve,
+ mock_saved_qc,
+ mock_form_data_sql,
+ mcp_server,
+ ):
+ """Test fallback to form_data path when saved query_context returns None."""
+ from fastmcp import Client
+
+ from superset.mcp_service.chart.chart_utils import (
+ DatasetValidationResult,
+ )
+
+ mock_chart = Mock()
+ mock_chart.id = 20
+ mock_chart.slice_name = "Revenue"
+ mock_chart.viz_type = "bar"
+ mock_find.return_value = mock_chart
+
+ mock_validate.return_value = DatasetValidationResult(
+ is_valid=True, dataset_id=2, dataset_name="revenue", warnings=[]
+ )
+ mock_resolve.return_value = ({"metrics": ["sum_revenue"]}, False)
+ mock_saved_qc.return_value = None
+ mock_form_data_sql.return_value = ChartSql(
+ chart_id=20,
+ chart_name="Revenue",
+ sql="SELECT SUM(revenue) FROM orders",
+ language="sql",
+ datasource_name="orders",
+ )
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "get_chart_sql", {"request": {"identifier": 20}}
+ )
+
+ data = result.structured_content.get("result", result.structured_content)
+ assert "SELECT SUM(revenue) FROM orders" in data["sql"]
+ mock_form_data_sql.assert_called_once()
+
+ @patch.object(_get_chart_sql_mod, "validate_chart_dataset")
+ @patch.object(_get_chart_sql_mod, "_find_chart_by_identifier")
+ @pytest.mark.asyncio
+ async def test_dataset_not_accessible(self, mock_find, mock_validate, mcp_server):
+ """Test that inaccessible dataset returns error."""
+ from fastmcp import Client
+
+ from superset.mcp_service.chart.chart_utils import (
+ DatasetValidationResult,
+ )
+
+ mock_chart = Mock()
+ mock_chart.id = 30
+ mock_chart.slice_name = "Restricted"
+ mock_chart.viz_type = "table"
+ mock_find.return_value = mock_chart
+
+ mock_validate.return_value = DatasetValidationResult(
+ is_valid=False,
+ dataset_id=3,
+ dataset_name="secret_ds",
+ warnings=[],
+ error="Access denied to dataset secret_ds",
+ )
+
+ async with Client(mcp_server) as client:
+ result = await client.call_tool(
+ "get_chart_sql", {"request": {"identifier": 30}}
+ )
+
+ data = result.structured_content.get("result", result.structured_content)
+ assert data["error_type"] == "DatasetNotAccessible"
+ assert "Access denied" in data["error"]