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/07/01 16:56:56 UTC
(superset) branch aminghadersohi/mcp-semantic-layer-tools updated: fix(mcp): address Codex review findings on semantic layer tools
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch aminghadersohi/mcp-semantic-layer-tools
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/aminghadersohi/mcp-semantic-layer-tools by this push:
new fa0e3b733ea fix(mcp): address Codex review findings on semantic layer tools
fa0e3b733ea is described below
commit fa0e3b733eadabb3032079914122af86d115af5b
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Wed Jul 1 16:56:08 2026 +0000
fix(mcp): address Codex review findings on semantic layer tools
- BLOCKER: add view.raise_for_access() on all external SemanticView
paths; use SemanticViewDAO.find_accessible() for list-all to filter
at SQL level, mirroring DatasourceDAO.build_semantic_view_query()
- MEDIUM: validate requested metrics/dimensions/filters/order_by against
the resolved datasource in get_table, matching query_dataset behavior;
extract shared validate_names() to mcp_service/utils/query_utils.py
- MEDIUM: fix N+1 in list_metrics find_all path by using _apply_base_filter
with subqueryload eager options instead of lazy-loaded find_all()
- NIT: extract _format_columns to format_data_columns() in response_utils.py;
query_dataset now shares the same implementation
- NIT: SemanticLayerError now extends MCPBaseError for a consistent error
shape across all MCP tools
- NIT: fix file permissions (644) on new semantic layer Python files
---
.pre-commit-config.yaml | 0
superset/daos/semantic_layer.py | 23 ++++++
superset/mcp_service/__init__.py | 0
superset/mcp_service/__main__.py | 0
superset/mcp_service/annotation_layer/__init__.py | 0
superset/mcp_service/annotation_layer/schemas.py | 0
.../mcp_service/annotation_layer/tool/__init__.py | 0
.../tool/get_annotation_layer_info.py | 0
.../tool/get_layer_annotation_info.py | 0
.../tool/list_annotation_layers.py | 0
.../tool/list_layer_annotations.py | 0
superset/mcp_service/app.py | 0
superset/mcp_service/auth.py | 0
superset/mcp_service/chart/chart_helpers.py | 0
superset/mcp_service/chart/chart_utils.py | 0
.../chart/prompts/create_chart_guided.py | 0
superset/mcp_service/chart/schemas.py | 0
superset/mcp_service/chart/tool/generate_chart.py | 0
superset/mcp_service/chart/tool/get_chart_data.py | 0
superset/mcp_service/chart/tool/get_chart_info.py | 0
.../mcp_service/chart/tool/get_chart_preview.py | 0
.../chart/tool/get_chart_type_schema.py | 0
superset/mcp_service/chart/tool/list_charts.py | 0
superset/mcp_service/chart/tool/update_chart.py | 0
.../mcp_service/chart/tool/update_chart_preview.py | 0
.../chart/validation/dataset_validator.py | 0
superset/mcp_service/chart/validation/pipeline.py | 0
.../chart/validation/runtime/__init__.py | 0
.../validation/runtime/chart_type_suggester.py | 0
.../chart/validation/schema_validator.py | 0
superset/mcp_service/common/schema_discovery.py | 0
superset/mcp_service/composite_token_verifier.py | 0
superset/mcp_service/constants.py | 0
superset/mcp_service/dashboard/schemas.py | 0
superset/mcp_service/dashboard/tool/__init__.py | 0
.../tool/add_chart_to_existing_dashboard.py | 0
.../dashboard/tool/duplicate_dashboard.py | 0
.../dashboard/tool/generate_dashboard.py | 0
.../dashboard/tool/get_dashboard_datasets.py | 0
.../dashboard/tool/get_dashboard_info.py | 0
.../dashboard/tool/get_dashboard_layout.py | 0
.../mcp_service/dashboard/tool/list_dashboards.py | 0
.../dashboard/tool/manage_native_filters.py | 0
.../dashboard/tool/remove_chart_from_dashboard.py | 0
.../mcp_service/dashboard/tool/update_dashboard.py | 0
superset/mcp_service/database/schemas.py | 0
superset/mcp_service/dataset/schemas.py | 0
superset/mcp_service/dataset/tool/__init__.py | 0
.../mcp_service/dataset/tool/create_dataset.py | 0
.../dataset/tool/create_virtual_dataset.py | 0
.../mcp_service/dataset/tool/get_dataset_info.py | 0
superset/mcp_service/dataset/tool/list_datasets.py | 0
superset/mcp_service/dataset/tool/query_dataset.py | 72 ++---------------
.../mcp_service/docs/tool-search-optimization.md | 0
superset/mcp_service/explore/schemas.py | 0
.../explore/tool/generate_explore_link.py | 0
superset/mcp_service/jwt_verifier.py | 0
superset/mcp_service/mcp_config.py | 0
superset/mcp_service/mcp_core.py | 0
superset/mcp_service/middleware.py | 0
superset/mcp_service/privacy.py | 0
superset/mcp_service/query/__init__.py | 0
superset/mcp_service/query/schemas.py | 0
superset/mcp_service/query/tool/__init__.py | 0
superset/mcp_service/query/tool/get_query_info.py | 0
superset/mcp_service/query/tool/list_queries.py | 0
superset/mcp_service/report/__init__.py | 0
superset/mcp_service/report/schemas.py | 0
superset/mcp_service/report/tool/__init__.py | 0
.../mcp_service/report/tool/get_report_info.py | 0
superset/mcp_service/report/tool/list_reports.py | 0
superset/mcp_service/rls/__init__.py | 0
superset/mcp_service/rls/schemas.py | 0
superset/mcp_service/rls/tool/__init__.py | 0
.../mcp_service/rls/tool/get_rls_filter_info.py | 0
superset/mcp_service/rls/tool/list_rls_filters.py | 0
superset/mcp_service/role/__init__.py | 0
superset/mcp_service/role/schemas.py | 0
superset/mcp_service/role/tool/__init__.py | 0
superset/mcp_service/role/tool/get_role_info.py | 0
superset/mcp_service/role/tool/list_roles.py | 0
superset/mcp_service/saved_query/__init__.py | 0
superset/mcp_service/saved_query/schemas.py | 0
superset/mcp_service/saved_query/tool/__init__.py | 0
.../saved_query/tool/get_saved_query_info.py | 0
.../saved_query/tool/list_saved_queries.py | 0
.../mcp_service/screenshot/webdriver_config.py | 0
superset/mcp_service/screenshot/webdriver_pool.py | 0
superset/mcp_service/semantic_layer/__init__.py | 0
superset/mcp_service/semantic_layer/schemas.py | 5 +-
.../mcp_service/semantic_layer/tool/__init__.py | 0
.../tool/get_compatible_dimensions.py | 9 +++
.../semantic_layer/tool/get_compatible_metrics.py | 9 +++
.../mcp_service/semantic_layer/tool/get_table.py | 94 ++++++++++++----------
.../semantic_layer/tool/list_metrics.py | 16 +++-
superset/mcp_service/server.py | 0
superset/mcp_service/sql_lab/schemas.py | 0
.../sql_lab/tool/open_sql_lab_with_context.py | 0
superset/mcp_service/storage.py | 0
superset/mcp_service/system/schemas.py | 0
superset/mcp_service/system/tool/__init__.py | 0
superset/mcp_service/system/tool/find_users.py | 0
superset/mcp_service/system/tool/get_schema.py | 0
superset/mcp_service/tag/__init__.py | 0
superset/mcp_service/tag/schemas.py | 0
superset/mcp_service/tag/tool/__init__.py | 0
superset/mcp_service/tag/tool/get_tag_info.py | 0
superset/mcp_service/tag/tool/list_tags.py | 0
superset/mcp_service/task/__init__.py | 0
superset/mcp_service/task/schemas.py | 0
superset/mcp_service/task/tool/__init__.py | 0
superset/mcp_service/task/tool/get_task_info.py | 0
superset/mcp_service/task/tool/list_tasks.py | 0
superset/mcp_service/user/__init__.py | 0
superset/mcp_service/user/schemas.py | 0
superset/mcp_service/user/tool/__init__.py | 0
superset/mcp_service/user/tool/get_user_info.py | 0
superset/mcp_service/user/tool/list_users.py | 0
superset/mcp_service/utils/error_sanitization.py | 0
superset/mcp_service/utils/permissions_utils.py | 0
.../tool/__init__.py => utils/query_utils.py} | 32 +++++---
superset/mcp_service/utils/response_utils.py | 47 ++++++++++-
superset/mcp_service/utils/sanitization.py | 0
superset/mcp_service/utils/url_utils.py | 0
124 files changed, 183 insertions(+), 124 deletions(-)
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
old mode 100755
new mode 100644
diff --git a/superset/daos/semantic_layer.py b/superset/daos/semantic_layer.py
index 69f6af87ace..4d0296bd7b7 100644
--- a/superset/daos/semantic_layer.py
+++ b/superset/daos/semantic_layer.py
@@ -201,6 +201,29 @@ class SemanticViewDAO(BaseDAO[SemanticView], AbstractSemanticViewDAO):
for c in candidates
)
+ @classmethod
+ def find_accessible(cls) -> list[SemanticView]:
+ """Return all views the current user can access, filtered at SQL level.
+
+ Mirrors the permission filter in ``DatasourceDAO.build_semantic_view_query``
+ to avoid per-row Python-level access checks when listing all views.
+ """
+ from sqlalchemy import or_
+
+ query = db.session.query(SemanticView)
+ if not security_manager.can_access_all_datasources():
+ perms = security_manager.user_view_menu_names("datasource_access")
+ query = query.outerjoin(
+ SemanticLayer,
+ SemanticLayer.uuid == SemanticView.semantic_layer_uuid,
+ ).filter(
+ or_(
+ SemanticView.perm.in_(perms),
+ SemanticLayer.perm.in_(perms),
+ )
+ )
+ return query.all()
+
@classmethod
def find_by_name(cls, name: str, layer_uuid: str) -> SemanticView | None:
"""
diff --git a/superset/mcp_service/__init__.py b/superset/mcp_service/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/__main__.py b/superset/mcp_service/__main__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/__init__.py b/superset/mcp_service/annotation_layer/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/schemas.py b/superset/mcp_service/annotation_layer/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/__init__.py b/superset/mcp_service/annotation_layer/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/get_annotation_layer_info.py b/superset/mcp_service/annotation_layer/tool/get_annotation_layer_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/get_layer_annotation_info.py b/superset/mcp_service/annotation_layer/tool/get_layer_annotation_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/list_annotation_layers.py b/superset/mcp_service/annotation_layer/tool/list_annotation_layers.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/list_layer_annotations.py b/superset/mcp_service/annotation_layer/tool/list_layer_annotations.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/app.py b/superset/mcp_service/app.py
old mode 100755
new mode 100644
diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/chart_helpers.py b/superset/mcp_service/chart/chart_helpers.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/chart_utils.py b/superset/mcp_service/chart/chart_utils.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/prompts/create_chart_guided.py b/superset/mcp_service/chart/prompts/create_chart_guided.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/schemas.py b/superset/mcp_service/chart/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/generate_chart.py b/superset/mcp_service/chart/tool/generate_chart.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/get_chart_data.py b/superset/mcp_service/chart/tool/get_chart_data.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py b/superset/mcp_service/chart/tool/get_chart_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/get_chart_preview.py b/superset/mcp_service/chart/tool/get_chart_preview.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/get_chart_type_schema.py b/superset/mcp_service/chart/tool/get_chart_type_schema.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/list_charts.py b/superset/mcp_service/chart/tool/list_charts.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/update_chart.py b/superset/mcp_service/chart/tool/update_chart.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py b/superset/mcp_service/chart/tool/update_chart_preview.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/validation/dataset_validator.py b/superset/mcp_service/chart/validation/dataset_validator.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/validation/pipeline.py b/superset/mcp_service/chart/validation/pipeline.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/validation/runtime/__init__.py b/superset/mcp_service/chart/validation/runtime/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/validation/runtime/chart_type_suggester.py b/superset/mcp_service/chart/validation/runtime/chart_type_suggester.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/chart/validation/schema_validator.py b/superset/mcp_service/chart/validation/schema_validator.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/common/schema_discovery.py b/superset/mcp_service/common/schema_discovery.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/composite_token_verifier.py b/superset/mcp_service/composite_token_verifier.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/constants.py b/superset/mcp_service/constants.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/schemas.py b/superset/mcp_service/dashboard/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/__init__.py b/superset/mcp_service/dashboard/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/duplicate_dashboard.py b/superset/mcp_service/dashboard/tool/duplicate_dashboard.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py b/superset/mcp_service/dashboard/tool/generate_dashboard.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_datasets.py b/superset/mcp_service/dashboard/tool/get_dashboard_datasets.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_info.py b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_layout.py b/superset/mcp_service/dashboard/tool/get_dashboard_layout.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/list_dashboards.py b/superset/mcp_service/dashboard/tool/list_dashboards.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/manage_native_filters.py b/superset/mcp_service/dashboard/tool/manage_native_filters.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py b/superset/mcp_service/dashboard/tool/remove_chart_from_dashboard.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dashboard/tool/update_dashboard.py b/superset/mcp_service/dashboard/tool/update_dashboard.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/database/schemas.py b/superset/mcp_service/database/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/schemas.py b/superset/mcp_service/dataset/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/__init__.py b/superset/mcp_service/dataset/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/create_dataset.py b/superset/mcp_service/dataset/tool/create_dataset.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/create_virtual_dataset.py b/superset/mcp_service/dataset/tool/create_virtual_dataset.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/get_dataset_info.py b/superset/mcp_service/dataset/tool/get_dataset_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/list_datasets.py b/superset/mcp_service/dataset/tool/list_datasets.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/dataset/tool/query_dataset.py b/superset/mcp_service/dataset/tool/query_dataset.py
index da7f2d227f6..631554e5cea 100644
--- a/superset/mcp_service/dataset/tool/query_dataset.py
+++ b/superset/mcp_service/dataset/tool/query_dataset.py
@@ -22,7 +22,6 @@ Query a dataset using its semantic layer (saved metrics, calculated columns,
dimensions) without requiring a saved chart.
"""
-import difflib
import logging
import time
from typing import Any
@@ -35,7 +34,7 @@ from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.commands.exceptions import CommandException
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.chart.schemas import PerformanceMetadata
from superset.mcp_service.dataset.schemas import (
DatasetError,
QueryDatasetFilter,
@@ -50,6 +49,8 @@ from superset.mcp_service.privacy import (
from superset.mcp_service.utils import _is_uuid
from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
from superset.mcp_service.utils.oauth2_utils import build_oauth2_redirect_message
+from superset.mcp_service.utils.query_utils import validate_names
+from superset.mcp_service.utils.response_utils import format_data_columns
logger = logging.getLogger(__name__)
@@ -80,26 +81,6 @@ def _resolve_dataset(identifier: int | str, eager_options: list[Any]) -> Any | N
return None
-def _validate_names(
- requested: list[str],
- valid: set[str],
- kind: str,
-) -> list[str]:
- """Return list of error messages for names not found in *valid*.
-
- Includes close-match suggestions when available.
- """
- errors: list[str] = []
- for name in requested:
- if name not in valid:
- suggestions = difflib.get_close_matches(name, valid, n=3, cutoff=0.6)
- msg = f"Unknown {kind}: '{name}'"
- if suggestions:
- msg += f". Did you mean: {', '.join(suggestions)}?"
- errors.append(msg)
- return errors
-
-
@requires_data_model_metadata_access
@tool(
tags=["data"],
@@ -210,21 +191,21 @@ async def query_dataset( # noqa: C901
validation_errors: list[str] = []
validation_errors.extend(
- _validate_names(request.columns, valid_columns, "column")
+ validate_names(request.columns, valid_columns, "column")
)
validation_errors.extend(
- _validate_names(request.metrics, valid_metrics, "metric")
+ validate_names(request.metrics, valid_metrics, "metric")
)
# Validate filter column names against dataset columns
filter_cols = [f.col for f in request.filters]
validation_errors.extend(
- _validate_names(filter_cols, valid_columns, "filter column")
+ validate_names(filter_cols, valid_columns, "filter column")
)
# Validate order_by names against columns + metrics
if request.order_by:
valid_orderby = valid_columns | valid_metrics
validation_errors.extend(
- _validate_names(request.order_by, valid_orderby, "order_by")
+ validate_names(request.order_by, valid_orderby, "order_by")
)
if validation_errors:
@@ -379,44 +360,7 @@ async def query_dataset( # noqa: C901
warnings=warnings,
)
- # Build column metadata in a single pass per column.
- # Cap stats computation at STATS_SAMPLE rows to avoid O(rows*cols)
- # overhead on large result sets (row_limit allows up to 50k).
- stats_sample_size = 5000
- stats_rows = data[:stats_sample_size]
-
- columns_meta: list[DataColumn] = []
- for col_name in raw_columns:
- sample_values = [
- row.get(col_name) for row in data[:3] if row.get(col_name) is not None
- ]
- data_type = "string"
- if sample_values:
- if all(isinstance(v, bool) for v in sample_values):
- data_type = "boolean"
- elif all(isinstance(v, (int, float)) for v in sample_values):
- data_type = "numeric"
-
- # Compute null_count and unique non-null values in one pass
- null_count = 0
- unique_vals: set[str] = set()
- for row in stats_rows:
- val = row.get(col_name)
- if val is None:
- null_count += 1
- else:
- unique_vals.add(str(val))
-
- columns_meta.append(
- DataColumn(
- name=col_name,
- display_name=col_name.replace("_", " ").title(),
- data_type=data_type,
- sample_values=sample_values[:3],
- null_count=null_count,
- unique_count=len(unique_vals),
- )
- )
+ columns_meta = format_data_columns(data, raw_columns)
cache_status = get_cache_status_from_result(
query_result, force_refresh=request.force_refresh
diff --git a/superset/mcp_service/docs/tool-search-optimization.md b/superset/mcp_service/docs/tool-search-optimization.md
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/explore/schemas.py b/superset/mcp_service/explore/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/explore/tool/generate_explore_link.py b/superset/mcp_service/explore/tool/generate_explore_link.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/jwt_verifier.py b/superset/mcp_service/jwt_verifier.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/mcp_config.py b/superset/mcp_service/mcp_config.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/mcp_core.py b/superset/mcp_service/mcp_core.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/middleware.py b/superset/mcp_service/middleware.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/privacy.py b/superset/mcp_service/privacy.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/query/__init__.py b/superset/mcp_service/query/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/query/schemas.py b/superset/mcp_service/query/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/query/tool/__init__.py b/superset/mcp_service/query/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/query/tool/get_query_info.py b/superset/mcp_service/query/tool/get_query_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/query/tool/list_queries.py b/superset/mcp_service/query/tool/list_queries.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/report/__init__.py b/superset/mcp_service/report/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/report/schemas.py b/superset/mcp_service/report/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/report/tool/__init__.py b/superset/mcp_service/report/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/report/tool/get_report_info.py b/superset/mcp_service/report/tool/get_report_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/report/tool/list_reports.py b/superset/mcp_service/report/tool/list_reports.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/rls/__init__.py b/superset/mcp_service/rls/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/rls/schemas.py b/superset/mcp_service/rls/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/rls/tool/__init__.py b/superset/mcp_service/rls/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/rls/tool/get_rls_filter_info.py b/superset/mcp_service/rls/tool/get_rls_filter_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/rls/tool/list_rls_filters.py b/superset/mcp_service/rls/tool/list_rls_filters.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/role/__init__.py b/superset/mcp_service/role/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/role/schemas.py b/superset/mcp_service/role/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/role/tool/__init__.py b/superset/mcp_service/role/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/role/tool/get_role_info.py b/superset/mcp_service/role/tool/get_role_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/role/tool/list_roles.py b/superset/mcp_service/role/tool/list_roles.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/saved_query/__init__.py b/superset/mcp_service/saved_query/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/saved_query/schemas.py b/superset/mcp_service/saved_query/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/saved_query/tool/__init__.py b/superset/mcp_service/saved_query/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/saved_query/tool/get_saved_query_info.py b/superset/mcp_service/saved_query/tool/get_saved_query_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/saved_query/tool/list_saved_queries.py b/superset/mcp_service/saved_query/tool/list_saved_queries.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/screenshot/webdriver_config.py b/superset/mcp_service/screenshot/webdriver_config.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/screenshot/webdriver_pool.py b/superset/mcp_service/screenshot/webdriver_pool.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/semantic_layer/__init__.py b/superset/mcp_service/semantic_layer/__init__.py
old mode 100755
new mode 100644
diff --git a/superset/mcp_service/semantic_layer/schemas.py b/superset/mcp_service/semantic_layer/schemas.py
old mode 100755
new mode 100644
index 9104ddd8cc0..a031b28385a
--- a/superset/mcp_service/semantic_layer/schemas.py
+++ b/superset/mcp_service/semantic_layer/schemas.py
@@ -25,17 +25,16 @@ from pydantic import BaseModel, Field
from superset.mcp_service.chart.schemas import DataColumn, PerformanceMetadata
from superset.mcp_service.common.cache_schemas import CacheStatus
+from superset.mcp_service.common.error_schemas import MCPBaseError
# ---------------------------------------------------------------------------
# Shared error schema
# ---------------------------------------------------------------------------
-class SemanticLayerError(BaseModel):
+class SemanticLayerError(MCPBaseError):
"""Error response returned by semantic layer tools."""
- error: str
- error_type: str
success: Literal[False] = False
@classmethod
diff --git a/superset/mcp_service/semantic_layer/tool/__init__.py b/superset/mcp_service/semantic_layer/tool/__init__.py
old mode 100755
new mode 100644
diff --git a/superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py b/superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py
old mode 100755
new mode 100644
index a7abdd2191b..fcc908edd53
--- a/superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py
+++ b/superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py
@@ -163,6 +163,7 @@ async def get_compatible_dimensions(
# External semantic view path
# ------------------------------------------------------------------
from superset.daos.semantic_layer import SemanticViewDAO
+ from superset.exceptions import SupersetSecurityException
view_id: int = request.view_id # type: ignore[assignment]
with event_logger.log_context(action="mcp.get_compatible_dimensions.external"):
@@ -174,6 +175,14 @@ async def get_compatible_dimensions(
error_type="NotFound",
)
+ try:
+ view.raise_for_access()
+ except SupersetSecurityException as ex:
+ return SemanticLayerError.create(
+ error=str(ex.error.message),
+ error_type="AccessDenied",
+ )
+
compatible_names = view.get_compatible_dimensions(
request.selected_metrics,
request.selected_dimensions,
diff --git a/superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py b/superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py
old mode 100755
new mode 100644
index 83f6579a627..fae3be35c67
--- a/superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py
+++ b/superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py
@@ -157,6 +157,7 @@ async def get_compatible_metrics(
# External semantic view path
# ------------------------------------------------------------------
from superset.daos.semantic_layer import SemanticViewDAO
+ from superset.exceptions import SupersetSecurityException
view_id: int = request.view_id # type: ignore[assignment]
with event_logger.log_context(action="mcp.get_compatible_metrics.external"):
@@ -168,6 +169,14 @@ async def get_compatible_metrics(
error_type="NotFound",
)
+ try:
+ view.raise_for_access()
+ except SupersetSecurityException as ex:
+ return SemanticLayerError.create(
+ error=str(ex.error.message),
+ error_type="AccessDenied",
+ )
+
compatible_names = view.get_compatible_metrics(
request.selected_metrics,
request.selected_dimensions,
diff --git a/superset/mcp_service/semantic_layer/tool/get_table.py b/superset/mcp_service/semantic_layer/tool/get_table.py
old mode 100755
new mode 100644
index 7f3eb3ecdfa..d19dcff77fa
--- a/superset/mcp_service/semantic_layer/tool/get_table.py
+++ b/superset/mcp_service/semantic_layer/tool/get_table.py
@@ -32,7 +32,7 @@ from superset_core.mcp.decorators import tool, ToolAnnotations
from superset.commands.exceptions import CommandException
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.chart.schemas import PerformanceMetadata
from superset.mcp_service.privacy import (
DATA_MODEL_METADATA_ERROR_TYPE,
requires_data_model_metadata_access,
@@ -45,6 +45,8 @@ from superset.mcp_service.semantic_layer.schemas import (
)
from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
from superset.mcp_service.utils.oauth2_utils import build_oauth2_redirect_message
+from superset.mcp_service.utils.query_utils import validate_names
+from superset.mcp_service.utils.response_utils import format_data_columns
logger = logging.getLogger(__name__)
@@ -78,44 +80,6 @@ def _build_query_dict(
return query_dict
-def _format_columns(
- data: list[dict[str, Any]], raw_columns: list[str]
-) -> list[DataColumn]:
- stats_rows = data[:5000]
- columns_meta: list[DataColumn] = []
- for col_name in raw_columns:
- sample_values = [
- row.get(col_name) for row in data[:3] if row.get(col_name) is not None
- ]
- data_type = "string"
- if sample_values:
- if all(isinstance(v, bool) for v in sample_values):
- data_type = "boolean"
- elif all(isinstance(v, (int, float)) for v in sample_values):
- data_type = "numeric"
-
- null_count = 0
- unique_vals: set[str] = set()
- for row in stats_rows:
- val = row.get(col_name)
- if val is None:
- null_count += 1
- else:
- unique_vals.add(str(val))
-
- columns_meta.append(
- DataColumn(
- name=col_name,
- display_name=col_name.replace("_", " ").title(),
- data_type=data_type,
- sample_values=sample_values[:3],
- null_count=null_count,
- unique_count=len(unique_vals),
- )
- )
- return columns_meta
-
-
@tool(
tags=["data", "semantic"],
class_permission_name="Dataset",
@@ -208,10 +172,12 @@ async def get_table( # noqa: C901
# ------------------------------------------------------------------
# Resolve datasource for metadata (time column, display name)
# ------------------------------------------------------------------
- await ctx.report_progress(1, 4, "Resolving data source")
+ await ctx.report_progress(1, 5, "Resolving data source")
display_name: str = f"{'Dataset' if is_builtin else 'View'} {datasource_id}"
time_col: str | None = request.time_column
warnings: list[str] = []
+ valid_columns: set[str] = set()
+ valid_metrics: set[str] = set()
if is_builtin:
from sqlalchemy.orm import subqueryload
@@ -233,6 +199,8 @@ async def get_table( # noqa: C901
error_type="NotFound",
)
display_name = dataset.table_name
+ valid_columns = {c.column_name for c in dataset.columns}
+ valid_metrics = {m.metric_name for m in dataset.metrics}
if time_col is None and request.time_range:
time_col = getattr(dataset, "main_dttm_col", None)
if not time_col:
@@ -245,6 +213,7 @@ async def get_table( # noqa: C901
)
else:
from superset.daos.semantic_layer import SemanticViewDAO
+ from superset.exceptions import SupersetSecurityException
with event_logger.log_context(action="mcp.get_table.resolve_view"):
view = SemanticViewDAO.find_by_id(datasource_id)
@@ -253,7 +222,16 @@ async def get_table( # noqa: C901
error=f"No semantic view found with id: {request.view_id}.",
error_type="NotFound",
)
+ try:
+ view.raise_for_access()
+ except SupersetSecurityException as ex:
+ return SemanticLayerError.create(
+ error=str(ex.error.message),
+ error_type="AccessDenied",
+ )
display_name = view.name
+ valid_columns = {c.column_name for c in view.columns}
+ valid_metrics = {m.metric_name for m in view.metrics}
if time_col is None and request.time_range:
# Use first datetime dimension as the time column
dttm_cols = [c for c in view.columns if c.is_dttm]
@@ -266,14 +244,42 @@ async def get_table( # noqa: C901
)
time_col = None
+ # ------------------------------------------------------------------
+ # Validate requested metrics and dimensions against the datasource
+ # ------------------------------------------------------------------
+ await ctx.report_progress(2, 5, "Validating metrics and dimensions")
+ validation_errors: list[str] = []
+ validation_errors.extend(
+ validate_names(request.dimensions, valid_columns, "dimension")
+ )
+ validation_errors.extend(
+ validate_names(request.metrics, valid_metrics, "metric")
+ )
+ filter_cols = [f.col for f in request.filters]
+ validation_errors.extend(
+ validate_names(filter_cols, valid_columns, "filter column")
+ )
+ if request.order_by:
+ valid_orderby = valid_columns | valid_metrics
+ validation_errors.extend(
+ validate_names(request.order_by, valid_orderby, "order_by")
+ )
+ if validation_errors:
+ error_msg = "; ".join(validation_errors)
+ await ctx.error("Validation failed: %s" % (error_msg,))
+ return SemanticLayerError.create(
+ error=error_msg,
+ error_type="ValidationError",
+ )
+
# ------------------------------------------------------------------
# Build and execute query
# ------------------------------------------------------------------
- await ctx.report_progress(2, 4, "Building query")
+ await ctx.report_progress(3, 5, "Building query")
query_dict = _build_query_dict(request, time_col)
await ctx.debug("Query dict: %s" % (sorted(query_dict.keys()),))
- await ctx.report_progress(3, 4, "Executing query")
+ await ctx.report_progress(4, 5, "Executing query")
start_time = time.time()
with event_logger.log_context(action="mcp.get_table.execute"):
@@ -299,7 +305,7 @@ async def get_table( # noqa: C901
# ------------------------------------------------------------------
# Format response
# ------------------------------------------------------------------
- await ctx.report_progress(4, 4, "Formatting results")
+ await ctx.report_progress(5, 5, "Formatting results")
query_result = result["queries"][0]
data = query_result.get("data", [])
raw_columns = query_result.get("colnames", [])
@@ -326,7 +332,7 @@ async def get_table( # noqa: C901
warnings=warnings,
)
- columns_meta = _format_columns(data, raw_columns)
+ columns_meta = format_data_columns(data, raw_columns)
cache_status = get_cache_status_from_result(
query_result, force_refresh=request.force_refresh
)
diff --git a/superset/mcp_service/semantic_layer/tool/list_metrics.py b/superset/mcp_service/semantic_layer/tool/list_metrics.py
old mode 100755
new mode 100644
index bc2ed41e5f5..fd6bb732ccb
--- a/superset/mcp_service/semantic_layer/tool/list_metrics.py
+++ b/superset/mcp_service/semantic_layer/tool/list_metrics.py
@@ -73,6 +73,7 @@ def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
from superset.connectors.sqla.models import SqlaTable
from superset.daos.dataset import DatasetDAO
+ from superset.extensions import db
with event_logger.log_context(action="mcp.list_metrics.builtin_query"):
if request.dataset_id is not None:
@@ -85,7 +86,13 @@ def _collect_builtin_metrics(request: ListMetricsRequest) -> list[MetricInfo]:
)
datasets = [dataset] if dataset else []
else:
- datasets = DatasetDAO.find_all()
+ # Use _apply_base_filter with explicit eager loading to avoid
+ # N+1 queries when iterating dataset.metrics / dataset.columns.
+ query = db.session.query(SqlaTable).options(
+ subqueryload(SqlaTable.columns),
+ subqueryload(SqlaTable.metrics),
+ )
+ datasets = DatasetDAO._apply_base_filter(query).all()
results: list[MetricInfo] = []
for dataset in datasets:
@@ -131,12 +138,17 @@ async def _collect_external_metrics(
view = SemanticViewDAO.find_by_id(request.view_id)
views = [view] if view else []
else:
- views = SemanticViewDAO.find_all()
+ # find_accessible filters at SQL level, avoiding a per-row
+ # Python permission check and the audit noise of raise_for_access.
+ views = SemanticViewDAO.find_accessible()
await ctx.debug("Found %d semantic views to scan for metrics" % len(views))
results: list[MetricInfo] = []
for view in views:
+ # raise_for_access must be called outside the broad except block below
+ # so that auth errors are never silently swallowed.
+ view.raise_for_access()
try:
raw_metrics = view.metrics
raw_cols = view.columns if request.include_compatible_dimensions else []
diff --git a/superset/mcp_service/server.py b/superset/mcp_service/server.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/sql_lab/schemas.py b/superset/mcp_service/sql_lab/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py b/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/storage.py b/superset/mcp_service/storage.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/system/schemas.py b/superset/mcp_service/system/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/system/tool/__init__.py b/superset/mcp_service/system/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/system/tool/find_users.py b/superset/mcp_service/system/tool/find_users.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/system/tool/get_schema.py b/superset/mcp_service/system/tool/get_schema.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/tag/__init__.py b/superset/mcp_service/tag/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/tag/schemas.py b/superset/mcp_service/tag/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/tag/tool/__init__.py b/superset/mcp_service/tag/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/tag/tool/get_tag_info.py b/superset/mcp_service/tag/tool/get_tag_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/tag/tool/list_tags.py b/superset/mcp_service/tag/tool/list_tags.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/task/__init__.py b/superset/mcp_service/task/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/task/schemas.py b/superset/mcp_service/task/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/task/tool/__init__.py b/superset/mcp_service/task/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/task/tool/get_task_info.py b/superset/mcp_service/task/tool/get_task_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/task/tool/list_tasks.py b/superset/mcp_service/task/tool/list_tasks.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/user/__init__.py b/superset/mcp_service/user/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/user/schemas.py b/superset/mcp_service/user/schemas.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/user/tool/__init__.py b/superset/mcp_service/user/tool/__init__.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/user/tool/get_user_info.py b/superset/mcp_service/user/tool/get_user_info.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/user/tool/list_users.py b/superset/mcp_service/user/tool/list_users.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/utils/error_sanitization.py b/superset/mcp_service/utils/error_sanitization.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/utils/permissions_utils.py b/superset/mcp_service/utils/permissions_utils.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/annotation_layer/tool/__init__.py b/superset/mcp_service/utils/query_utils.py
similarity index 54%
copy from superset/mcp_service/annotation_layer/tool/__init__.py
copy to superset/mcp_service/utils/query_utils.py
index 75bbed4e1f0..884ef6d83c7 100644
--- a/superset/mcp_service/annotation_layer/tool/__init__.py
+++ b/superset/mcp_service/utils/query_utils.py
@@ -15,14 +15,26 @@
# specific language governing permissions and limitations
# under the License.
-from .get_annotation_layer_info import get_annotation_layer_info
-from .get_layer_annotation_info import get_layer_annotation_info
-from .list_annotation_layers import list_annotation_layers
-from .list_layer_annotations import list_layer_annotations
+"""Shared query validation utilities for MCP tools."""
-__all__ = [
- "list_annotation_layers",
- "get_annotation_layer_info",
- "list_layer_annotations",
- "get_layer_annotation_info",
-]
+import difflib
+
+
+def validate_names(
+ requested: list[str],
+ valid: set[str],
+ kind: str,
+) -> list[str]:
+ """Return list of error messages for names not found in *valid*.
+
+ Includes close-match suggestions when available.
+ """
+ errors: list[str] = []
+ for name in requested:
+ if name not in valid:
+ suggestions = difflib.get_close_matches(name, valid, n=3, cutoff=0.6)
+ msg = f"Unknown {kind}: '{name}'"
+ if suggestions:
+ msg += f". Did you mean: {', '.join(suggestions)}?"
+ errors.append(msg)
+ return errors
diff --git a/superset/mcp_service/utils/response_utils.py b/superset/mcp_service/utils/response_utils.py
old mode 100644
new mode 100755
index 4171287ff8b..63e3023c91f
--- a/superset/mcp_service/utils/response_utils.py
+++ b/superset/mcp_service/utils/response_utils.py
@@ -57,10 +57,12 @@ Usage example::
from __future__ import annotations
from datetime import datetime
-from typing import Dict
+from typing import Any, Dict
import humanize
+from superset.mcp_service.chart.schemas import DataColumn
+
def humanize_timestamp(dt: datetime | None) -> str | None:
"""Convert a datetime to a humanized string like '2 hours ago'."""
@@ -161,3 +163,46 @@ class OmittedFieldsBuilder:
def build(self) -> Dict[str, str]:
"""Return the omission metadata dict."""
return dict(self._fields)
+
+
+def format_data_columns(
+ data: list[dict[str, Any]], raw_columns: list[str]
+) -> list[DataColumn]:
+ """Build column metadata from query result data.
+
+ Caps statistics at 5000 rows to avoid O(rows*cols) overhead on large
+ result sets.
+ """
+ stats_rows = data[:5000]
+ columns_meta: list[DataColumn] = []
+ for col_name in raw_columns:
+ sample_values = [
+ row.get(col_name) for row in data[:3] if row.get(col_name) is not None
+ ]
+ data_type = "string"
+ if sample_values:
+ if all(isinstance(v, bool) for v in sample_values):
+ data_type = "boolean"
+ elif all(isinstance(v, (int, float)) for v in sample_values):
+ data_type = "numeric"
+
+ null_count = 0
+ unique_vals: set[str] = set()
+ for row in stats_rows:
+ val = row.get(col_name)
+ if val is None:
+ null_count += 1
+ else:
+ unique_vals.add(str(val))
+
+ columns_meta.append(
+ DataColumn(
+ name=col_name,
+ display_name=col_name.replace("_", " ").title(),
+ data_type=data_type,
+ sample_values=sample_values[:3],
+ null_count=null_count,
+ unique_count=len(unique_vals),
+ )
+ )
+ return columns_meta
diff --git a/superset/mcp_service/utils/sanitization.py b/superset/mcp_service/utils/sanitization.py
old mode 100644
new mode 100755
diff --git a/superset/mcp_service/utils/url_utils.py b/superset/mcp_service/utils/url_utils.py
old mode 100644
new mode 100755