aminghadersohi opened a new pull request, #38388:
URL: https://github.com/apache/superset/pull/38388

   ### SUMMARY
   
   When the MCP `execute_sql` tool runs multi-statement SQL (e.g., `SELECT 
COUNT(*) FROM orders; SELECT SUM(revenue) FROM orders`), only the last 
data-bearing statement's results appear in the top-level `rows`/`columns` 
response fields. The earlier statement results are completely lost — only 
metadata (`original_sql`, `row_count`) is preserved in the `statements` array, 
with no actual row data.
   
   **Root cause**: The `_convert_to_response()` function in `execute_sql.py` 
iterates backwards through statements but only extracts data from the last one 
with a DataFrame.
   
   **Fix**:
   - Adds a `StatementData` Pydantic schema (rows + columns) and a `data` field 
on `StatementInfo` so each statement carries its own result data for 
data-bearing statements (e.g., SELECT).
   - Populates per-statement data in `_convert_to_response()` for every 
statement that has a DataFrame result.
   - Keeps the top-level `rows`/`columns` pointing to the last data-bearing 
statement for backward compatibility with single-statement consumers.
   - Adds a `multi_statement_warning` field to `ExecuteSqlResponse` when 
multiple data-bearing statements exist, directing the LLM to check the 
`statements` array for all results.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — backend-only change to MCP tool response schema.
   
   ### TESTING INSTRUCTIONS
   
   1. Run the updated unit tests:
      ```bash
      python -m pytest 
tests/unit_tests/mcp_service/sql_lab/tool/test_execute_sql.py -x -q
      ```
   2. All 16 tests should pass, including:
      - `test_execute_sql_multi_statement` — verifies per-statement `data` 
field and `multi_statement_warning` for two SELECT queries
      - `test_execute_sql_multi_statement_preserves_all_data` — regression test 
for the exact bug scenario (`SELECT COUNT(*)...; SELECT SUM(...)...`)
      - `test_execute_sql_multi_statement_set_then_select` — verifies SET + 
SELECT only populates data for the SELECT, no warning
      - `test_execute_sql_multi_statement_all_dml` — verifies DML-only queries 
have no per-statement data and no warning
   3. Verify backward compatibility: existing single-statement tests pass 
unchanged (top-level `rows`/`columns` still work).
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [x] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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]

Reply via email to