aminghadersohi opened a new pull request, #44949:
URL: https://github.com/apache/superset/pull/44949
## TL;DR
- Return SQL result rows once instead of repeating the final result in
statement metadata.
- Preserve top-level results and earlier multi-statement results.
- Avoid repeating unchanged SQL; retain transformed SQL when it differs.
### SUMMARY
`execute_sql` included the last data-bearing result both in top-level
`rows`/`columns` and in `statements[].data`, increasing response size without
adding information.
The top-level result stays unchanged. The corresponding statement's `data`
is `null`; earlier data-bearing statements retain their nested rows and
columns. This also handles trailing DML, empty results, and cached row lists.
Per-statement counts, truncation flags, and timing remain available.
`executed_sql` is `null` when it equals `original_sql`. Schema descriptions,
the multi-statement warning, and user documentation explain where each result
lives.
Scope: response conversion and schema documentation only. Query execution,
access checks, and caching behavior are unchanged.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable; no UI changes.
Before a single `SELECT 1 AS x`: both `rows` and `statements[0].data.rows`
contained `[{"x": 1}]`, and both SQL fields contained the same string.
After: `rows` contains `[{"x": 1}]`; `statements[0].data` and unchanged
`executed_sql` are `null`.
### TESTING INSTRUCTIONS
```bash
PYTHONPATH="$PWD/superset-core/src:$PWD" pytest -q \
tests/unit_tests/mcp_service/sql_lab/ \
tests/unit_tests/mcp_service/test_response_serialization.py
```
118 tests passed. The eight added regression cases cover single SELECTs,
empty/cached results, multiple identical SELECTs, trailing DML, and unchanged
versus transformed SQL. Before the fix, seven regression cases failed and the
transformed-SQL case passed. After the fix, all eight pass. Existing MCP client
tests verify the serialized single- and multi-statement response shape.
All changed-file pre-commit hooks passed, including mypy; explicit pylint
checks passed.
Manual verification: call `execute_sql` with `SELECT 1 AS x`, then with
`SELECT 1 AS x; SELECT 2 AS y`. Confirm the final result is only at the top
level, earlier results remain in `statements[].data`, and unchanged SQL is not
repeated.
#### Eval evidence
Local deterministic regression cases: 1/8 before → 8/8 after; affected SQL
Lab and serialization suite: 118/118 pass. End-to-end evaluations against a
deployed server were not run.
#### Cost & latency delta
Local conversion + JSON serialization benchmark, 100 iterations per fixture
(not an end-to-end latency measurement):
| Fixture | JSON bytes before → after | p50 ms before → after | p95 ms
before → after |
| --- | --- | --- | --- |
| 1 row | 456 → 372 | 0.207 → 0.198 | 0.350 → 0.287 |
| 1,000 rows | 16,446 → 8,370 | 3.070 → 3.020 | 4.155 → 3.364 |
The 1,000-row payload is 49.1% smaller. Model tokens, billing, and
time-to-first-token were not measured. No model or prompt changes.
### ADDITIONAL INFORMATION
Risk: clients reading the final result from `statements[].data` must use the
existing top-level rows/columns instead, and clients reading unchanged
`executed_sql` should fall back to `original_sql`. Earlier results and
transformed SQL remain available. Rollback is a revert; no migration or
configuration changes.
Review guidance: focus on selecting the last **data-bearing** statement
rather than the last executed statement, and retaining earlier results even
when their rows equal the final result.
- [ ] 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]
