aminghadersohi opened a new pull request, #37968:
URL: https://github.com/apache/superset/pull/37968
### SUMMARY
Fixes an edge case in `execute_sql` MCP tool where multi-statement queries
(e.g., `SET search_path TO sales; WITH cte AS (...) SELECT ...`) return
incorrect results. The `_convert_to_response` function previously always used
the first statement's data for backward compatibility, but when the first
statement is a `SET` command with no data, the response would incorrectly show
`affected_rows` instead of the actual query results.
The fix iterates from the last statement backward to find the first one with
`data is not None`, which correctly returns SELECT results even when preceded
by SET or other non-data-bearing statements. For DML-only queries (where no
statement has data), it uses the last statement's `row_count` for
`affected_rows`.
**Changes:**
- `superset/mcp_service/sql_lab/tool/execute_sql.py`: Changed
`_convert_to_response` to find the last data-bearing statement instead of
blindly using the first statement
- `tests/unit_tests/mcp_service/sql_lab/tool/test_execute_sql.py`: Updated
existing multi-statement test assertion, added two new test cases for `SET ...
; SELECT ...` and all-DML multi-statement queries
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - backend-only change
### TESTING INSTRUCTIONS
1. Run the unit tests:
```bash
pytest tests/unit_tests/mcp_service/sql_lab/ -x -v
```
2. All 15 tests should pass, including:
- `test_execute_sql_multi_statement` - two SELECT statements, response
uses last
- `test_execute_sql_multi_statement_set_then_select` - SET + SELECT,
response correctly returns SELECT data
- `test_execute_sql_multi_statement_all_dml` - SET + UPDATE, response
correctly returns affected_rows from last statement
### 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
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
