michael-s-molina commented on code in PR #36739:
URL: https://github.com/apache/superset/pull/36739#discussion_r2631309863
##########
superset/mcp_service/sql_lab/tool/execute_sql.py:
##########
@@ -52,36 +59,75 @@ async def execute_sql(request: ExecuteSqlRequest, ctx:
Context) -> ExecuteSqlRes
# Log SQL query details (truncated for security)
sql_preview = request.sql[:100] + "..." if len(request.sql) > 100 else
request.sql
await ctx.debug(
- "SQL query details: sql_preview=%r, sql_length=%s, has_parameters=%s"
+ "SQL query details: sql_preview=%r, sql_length=%s,
has_template_params=%s"
% (
sql_preview,
len(request.sql),
- bool(request.parameters),
+ bool(request.template_params),
)
)
logger.info("Executing SQL query on database ID: %s", request.database_id)
try:
- # Use the ExecuteSqlCore to handle all the logic
- sql_tool = ExecuteSqlCore(use_command_mode=False, logger=logger)
- result = sql_tool.run_tool(request)
+ # Import inside function to avoid initialization issues
+ from superset import db, security_manager
+ from superset.models.core import Database
+
+ # 1. Get database and check access
+ database =
db.session.query(Database).filter_by(id=request.database_id).first()
+ if not database:
+ raise SupersetErrorException(
+ SupersetError(
+ message=f"Database with ID {request.database_id} not
found",
+ error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ )
+
+ if not security_manager.can_access_database(database):
+ raise SupersetSecurityException(
+ SupersetError(
+ message=f"Access denied to database
{database.database_name}",
+
error_type=SupersetErrorType.DATABASE_SECURITY_ACCESS_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ )
+
+ # 2. Build QueryOptions
+ options = QueryOptions(
+ catalog=request.catalog,
+ schema=request.schema_name,
+ limit=request.limit,
+ timeout_seconds=request.timeout,
+ template_params=request.template_params,
+ dry_run=request.dry_run,
+ cache=CacheOptions(force_refresh=True), # No caching for MCP
Review Comment:
Currently, query caching is disabled. Is this what we want for MCP tools or
should queries be cached? I know that MCP has a middleware that caches tool
calls but this is different.
--
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]
