aminghadersohi commented on code in PR #41611: URL: https://github.com/apache/superset/pull/41611#discussion_r3508234330
########## superset/mcp_service/semantic_layer/tool/get_table.py: ########## @@ -0,0 +1,397 @@ +# 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_table + +Query a data source (built-in dataset or external semantic view) using +metric and dimension names, returning tabular results. +""" + +import logging +import time +from typing import Any + +from fastmcp import Context +from sqlalchemy.exc import SQLAlchemyError +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.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + GetTableRequest, + GetTableResponse, + SemanticLayerError, +) +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 + +logger = logging.getLogger(__name__) + + +def _build_query_dict( + request: GetTableRequest, + time_col: str | None, +) -> dict[str, Any]: + """Assemble the query dict for QueryContextFactory.""" + filters: list[dict[str, Any]] = [ + {"col": f.col, "op": f.op, "val": f.val} for f in request.filters + ] + if request.time_range and time_col: + filters.append( + {"col": time_col, "op": "TEMPORAL_RANGE", "val": request.time_range} + ) + + query_dict: dict[str, Any] = { + "filters": filters, + "columns": request.dimensions, + "metrics": request.metrics, + "row_limit": request.row_limit, + "order_desc": request.order_desc, + } + if time_col: + query_dict["granularity"] = time_col + if request.order_by: + query_dict["orderby"] = [ + (col, not request.order_desc) for col in request.order_by + ] + 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", + annotations=ToolAnnotations( + title="Get table", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_table( # noqa: C901 + request: GetTableRequest, + ctx: Context, +) -> GetTableResponse | SemanticLayerError: + """Query a data source using metrics and dimensions, returning tabular results. + + Works with both built-in datasets and external semantic views. The + ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response. + + Workflow: + 1. list_metrics -> discover metrics and their compatible_dimensions + 2. get_table -> query with chosen metrics and dimensions + + Example (built-in): + ```json + { + "dataset_id": 42, + "metrics": ["revenue"], + "dimensions": ["region", "product_category"], + "time_range": "Last 30 days", + "row_limit": 500 + } + ``` + + Example (external): + ```json + { + "view_id": 5, + "metrics": ["bookings"], + "dimensions": ["listing__country_name"], + "row_limit": 100 + } + ``` + """ + await ctx.info( + "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, " + "dimensions=%s, row_limit=%s" + % ( + request.dataset_id, + request.view_id, + request.metrics, + request.dimensions, + request.row_limit, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error=( + "Provide either dataset_id (built-in dataset) or view_id " + "(external semantic view). Both are in the list_metrics response." + ), + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + from superset.commands.chart.data.get_data_command import ChartDataCommand + from superset.common.query_context_factory import QueryContextFactory + + is_builtin = request.dataset_id is not None + datasource_type = "table" if is_builtin else "semantic_view" + if is_builtin: + assert request.dataset_id is not None + datasource_id = request.dataset_id + else: + assert request.view_id is not None + datasource_id = request.view_id + + # ------------------------------------------------------------------ + # Resolve datasource for metadata (time column, display name) + # ------------------------------------------------------------------ + await ctx.report_progress(1, 4, "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] = [] + + if is_builtin: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + with event_logger.log_context(action="mcp.get_table.resolve_dataset"): + dataset = DatasetDAO.find_by_id( + datasource_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + display_name = dataset.table_name + if time_col is None and request.time_range: + time_col = getattr(dataset, "main_dttm_col", None) + if not time_col: + return SemanticLayerError.create( + error=( + "time_range was provided but no temporal column is " + "configured. Set time_column explicitly." + ), + error_type="ValidationError", + ) + else: + from superset.daos.semantic_layer import SemanticViewDAO + + with event_logger.log_context(action="mcp.get_table.resolve_view"): + view = SemanticViewDAO.find_by_id(datasource_id) + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {request.view_id}.", + error_type="NotFound", + ) + display_name = view.name Review Comment: Fixed — `get_table` now calls `view.raise_for_access()` right after resolving the `SemanticView` and before reading `view.name`/`view.columns`/`view.metrics`. On denial it returns a distinct `AccessDenied` error type (see reply on the get_compatible_dimensions raise_for_access comment for why we kept AccessDenied separate from NotFound rather than collapsing them). See `get_table.py` lines 219-231. ########## superset/mcp_service/semantic_layer/tool/get_table.py: ########## @@ -0,0 +1,397 @@ +# 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_table + +Query a data source (built-in dataset or external semantic view) using +metric and dimension names, returning tabular results. +""" + +import logging +import time +from typing import Any + +from fastmcp import Context +from sqlalchemy.exc import SQLAlchemyError +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.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + GetTableRequest, + GetTableResponse, + SemanticLayerError, +) +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 + +logger = logging.getLogger(__name__) + + +def _build_query_dict( + request: GetTableRequest, + time_col: str | None, +) -> dict[str, Any]: + """Assemble the query dict for QueryContextFactory.""" + filters: list[dict[str, Any]] = [ + {"col": f.col, "op": f.op, "val": f.val} for f in request.filters + ] + if request.time_range and time_col: + filters.append( + {"col": time_col, "op": "TEMPORAL_RANGE", "val": request.time_range} + ) + + query_dict: dict[str, Any] = { + "filters": filters, + "columns": request.dimensions, + "metrics": request.metrics, + "row_limit": request.row_limit, + "order_desc": request.order_desc, + } + if time_col: + query_dict["granularity"] = time_col + if request.order_by: + query_dict["orderby"] = [ + (col, not request.order_desc) for col in request.order_by + ] + 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", + annotations=ToolAnnotations( + title="Get table", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_table( # noqa: C901 + request: GetTableRequest, + ctx: Context, +) -> GetTableResponse | SemanticLayerError: + """Query a data source using metrics and dimensions, returning tabular results. + + Works with both built-in datasets and external semantic views. The + ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response. + + Workflow: + 1. list_metrics -> discover metrics and their compatible_dimensions + 2. get_table -> query with chosen metrics and dimensions + + Example (built-in): + ```json + { + "dataset_id": 42, + "metrics": ["revenue"], + "dimensions": ["region", "product_category"], + "time_range": "Last 30 days", + "row_limit": 500 + } + ``` + + Example (external): + ```json + { + "view_id": 5, + "metrics": ["bookings"], + "dimensions": ["listing__country_name"], + "row_limit": 100 + } + ``` + """ + await ctx.info( + "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, " + "dimensions=%s, row_limit=%s" + % ( + request.dataset_id, + request.view_id, + request.metrics, + request.dimensions, + request.row_limit, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error=( + "Provide either dataset_id (built-in dataset) or view_id " + "(external semantic view). Both are in the list_metrics response." + ), + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + from superset.commands.chart.data.get_data_command import ChartDataCommand + from superset.common.query_context_factory import QueryContextFactory + + is_builtin = request.dataset_id is not None + datasource_type = "table" if is_builtin else "semantic_view" + if is_builtin: + assert request.dataset_id is not None + datasource_id = request.dataset_id + else: + assert request.view_id is not None + datasource_id = request.view_id + + # ------------------------------------------------------------------ + # Resolve datasource for metadata (time column, display name) + # ------------------------------------------------------------------ + await ctx.report_progress(1, 4, "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] = [] + + if is_builtin: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + with event_logger.log_context(action="mcp.get_table.resolve_dataset"): + dataset = DatasetDAO.find_by_id( + datasource_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + display_name = dataset.table_name + if time_col is None and request.time_range: + time_col = getattr(dataset, "main_dttm_col", None) + if not time_col: + return SemanticLayerError.create( + error=( + "time_range was provided but no temporal column is " + "configured. Set time_column explicitly." + ), + error_type="ValidationError", + ) Review Comment: Fixed — added explicit time_column validation for both the built-in and external paths. If the resolved `time_col` (explicit or inferred from `main_dttm_col`/first datetime dimension) doesn't exist on the datasource, or exists but isn't marked as a datetime column, `get_table` now returns a `ValidationError` with a clear message instead of passing it through to `QueryContextFactory`/`ChartDataCommand` and surfacing a cryptic downstream error. Added a regression test (`test_get_table_time_column_not_dttm_validation_error`). See `get_table.py` lines 203-224 and 249-273. ########## superset/mcp_service/semantic_layer/tool/get_table.py: ########## @@ -0,0 +1,397 @@ +# 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_table + +Query a data source (built-in dataset or external semantic view) using +metric and dimension names, returning tabular results. +""" + +import logging +import time +from typing import Any + +from fastmcp import Context +from sqlalchemy.exc import SQLAlchemyError +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.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + GetTableRequest, + GetTableResponse, + SemanticLayerError, +) +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 + +logger = logging.getLogger(__name__) + + +def _build_query_dict( + request: GetTableRequest, + time_col: str | None, +) -> dict[str, Any]: + """Assemble the query dict for QueryContextFactory.""" + filters: list[dict[str, Any]] = [ + {"col": f.col, "op": f.op, "val": f.val} for f in request.filters + ] + if request.time_range and time_col: + filters.append( + {"col": time_col, "op": "TEMPORAL_RANGE", "val": request.time_range} + ) + + query_dict: dict[str, Any] = { + "filters": filters, + "columns": request.dimensions, + "metrics": request.metrics, + "row_limit": request.row_limit, + "order_desc": request.order_desc, + } + if time_col: + query_dict["granularity"] = time_col + if request.order_by: + query_dict["orderby"] = [ + (col, not request.order_desc) for col in request.order_by + ] + 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", + annotations=ToolAnnotations( + title="Get table", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_table( # noqa: C901 + request: GetTableRequest, + ctx: Context, +) -> GetTableResponse | SemanticLayerError: + """Query a data source using metrics and dimensions, returning tabular results. + + Works with both built-in datasets and external semantic views. The + ``dataset_id`` or ``view_id`` comes from the ``list_metrics`` response. + + Workflow: + 1. list_metrics -> discover metrics and their compatible_dimensions + 2. get_table -> query with chosen metrics and dimensions + + Example (built-in): + ```json + { + "dataset_id": 42, + "metrics": ["revenue"], + "dimensions": ["region", "product_category"], + "time_range": "Last 30 days", + "row_limit": 500 + } + ``` + + Example (external): + ```json + { + "view_id": 5, + "metrics": ["bookings"], + "dimensions": ["listing__country_name"], + "row_limit": 100 + } + ``` + """ + await ctx.info( + "Starting get_table: dataset_id=%s, view_id=%s, metrics=%s, " + "dimensions=%s, row_limit=%s" + % ( + request.dataset_id, + request.view_id, + request.metrics, + request.dimensions, + request.row_limit, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error=( + "Provide either dataset_id (built-in dataset) or view_id " + "(external semantic view). Both are in the list_metrics response." + ), + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + from superset.commands.chart.data.get_data_command import ChartDataCommand + from superset.common.query_context_factory import QueryContextFactory + + is_builtin = request.dataset_id is not None + datasource_type = "table" if is_builtin else "semantic_view" + if is_builtin: + assert request.dataset_id is not None + datasource_id = request.dataset_id + else: + assert request.view_id is not None + datasource_id = request.view_id + + # ------------------------------------------------------------------ + # Resolve datasource for metadata (time column, display name) + # ------------------------------------------------------------------ + await ctx.report_progress(1, 4, "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] = [] + + if is_builtin: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + with event_logger.log_context(action="mcp.get_table.resolve_dataset"): + dataset = DatasetDAO.find_by_id( + datasource_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + display_name = dataset.table_name + if time_col is None and request.time_range: + time_col = getattr(dataset, "main_dttm_col", None) + if not time_col: + return SemanticLayerError.create( + error=( + "time_range was provided but no temporal column is " + "configured. Set time_column explicitly." + ), + error_type="ValidationError", + ) + else: + from superset.daos.semantic_layer import SemanticViewDAO + + with event_logger.log_context(action="mcp.get_table.resolve_view"): + view = SemanticViewDAO.find_by_id(datasource_id) + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {request.view_id}.", + error_type="NotFound", + ) + display_name = view.name + 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] + if dttm_cols: + time_col = dttm_cols[0].column_name + else: + warnings.append( + "time_range provided but no datetime dimension found; " + "time filter will not be applied." + ) + time_col = None + + # ------------------------------------------------------------------ + # Build and execute query + # ------------------------------------------------------------------ + await ctx.report_progress(2, 4, "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") + start_time = time.time() + + with event_logger.log_context(action="mcp.get_table.execute"): + factory = QueryContextFactory() + query_context = factory.create( + datasource={"id": datasource_id, "type": datasource_type}, + queries=[query_dict], + form_data={}, + force=not request.use_cache or request.force_refresh, + ) + command = ChartDataCommand(query_context) + command.validate() + result = command.run() + + query_duration_ms = int((time.time() - start_time) * 1000) + + if not result or "queries" not in result or not result["queries"]: + return SemanticLayerError.create( + error="Query returned no results.", + error_type="EmptyQuery", + ) + + # ------------------------------------------------------------------ + # Format response + # ------------------------------------------------------------------ + await ctx.report_progress(4, 4, "Formatting results") + query_result = result["queries"][0] + data = query_result.get("data", []) + raw_columns = query_result.get("colnames", []) + + if not data: + return GetTableResponse( + columns=[], + data=[], + row_count=0, + total_rows=0, + summary=f"'{display_name}': query returned no data.", + source="builtin" if is_builtin else "external", + dataset_id=request.dataset_id, + dataset_name=display_name if is_builtin else None, + view_id=request.view_id, + view_name=display_name if not is_builtin else None, + performance=PerformanceMetadata( + query_duration_ms=query_duration_ms, + cache_status="no_data", + ), + cache_status=get_cache_status_from_result( + query_result, force_refresh=request.force_refresh + ), + warnings=warnings, + ) + + columns_meta = _format_columns(data, raw_columns) + cache_status = get_cache_status_from_result( + query_result, force_refresh=request.force_refresh + ) + cache_label = "cached" if cache_status and cache_status.cache_hit else "fresh" + summary = ( + f"'{display_name}': {len(data)} rows, " + f"{len(raw_columns)} columns ({cache_label})." + ) + + await ctx.info( + "get_table complete: rows=%d, columns=%d, duration=%dms" + % (len(data), len(raw_columns), query_duration_ms) + ) + + return GetTableResponse( + columns=columns_meta, + data=data, + row_count=len(data), + total_rows=query_result.get("rowcount"), + summary=summary, + source="builtin" if is_builtin else "external", + dataset_id=request.dataset_id, + dataset_name=display_name if is_builtin else None, + view_id=request.view_id, + view_name=display_name if not is_builtin else None, + performance=PerformanceMetadata( + query_duration_ms=query_duration_ms, + cache_status=cache_label, + ), + cache_status=cache_status, + warnings=warnings, + ) + + except OAuth2RedirectError as exc: + redirect_msg = build_oauth2_redirect_message(exc) + await ctx.error("OAuth2 redirect required: %s" % redirect_msg) + return SemanticLayerError.create( + error=redirect_msg, + error_type="OAuth2Redirect", + ) + + except OAuth2Error as exc: + await ctx.error("OAuth2 error: %s" % str(exc)) + return SemanticLayerError.create( + error=f"OAuth2 authentication error: {exc}", + error_type="OAuth2Error", + ) + + except (CommandException, SupersetException) as exc: + await ctx.error("Query failed: %s" % str(exc)) + return SemanticLayerError.create( + error=f"Query execution failed: {exc}", + error_type="QueryError", + ) + + except SQLAlchemyError as exc: + await ctx.error("Database error: %s" % str(exc)) + return SemanticLayerError.create( + error=f"Database error: {exc}", + error_type="DatabaseError", + ) + + except Exception as exc: + logger.exception( + "Unexpected error in get_table: %s: %s", type(exc).__name__, str(exc) + ) + await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, str(exc))) + raise Review Comment: Fixed — the outer `except Exception` in `get_table` now returns a typed `SemanticLayerError(error_type="InternalError")` instead of re-raising, matching the other error branches (OAuth2, CommandException, SQLAlchemyError) already in this tool. See `get_table.py` lines 398-406. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
