eschutho opened a new pull request, #43888:
URL: https://github.com/apache/superset/pull/43888
### SUMMARY
Running **Estimate cost** in SQL Lab against a Presto/Trino table that lacks
computed statistics currently crashes with an opaque `500
GENERIC_BACKEND_ERROR` instead of a clean, typed error.
**Root cause.** `QueryEstimationCommand.run()`
(`superset/commands/sql_lab/estimate.py`) calls
`db_engine_spec.estimate_query_cost(...)`. For Presto/Trino,
`PrestoBaseEngineSpec.estimate_statement_cost` runs `EXPLAIN (TYPE IO, FORMAT
JSON) <statement>` and does `json.loads(cursor.fetchone()[0])` using the
simplejson-backed `superset.utils.json` (`allow_nan=False`). Trino/Presto
represent unknown/uncollected cost statistics as a **literal `NaN` token** in
that JSON (e.g. `{"estimate": {"outputRowCount": NaN, ...}}`), which happens
for any table without an `ANALYZE`. `allow_nan=False` then makes `json.loads`
raise a raw `simplejson.errors.JSONDecodeError`.
That exception is **not caught** anywhere in the call chain:
- `BaseEngineSpec.estimate_query_cost` calls `estimate_statement_cost` with
no guard;
- `QueryEstimationCommand.run()` only wraps the call in `try/except
SupersetTimeoutException`;
- `SqlLabRestApi.estimate_query_cost` has no `try/except`.
So it falls through to Flask's global `@app.errorhandler(Exception)` and
leaks to the user as an unhandled 500.
### FIX
Add a sibling `except json.JSONDecodeError` block to the existing
`try/except` around the `estimate_query_cost(...)` call, converting it to a
typed `SupersetErrorException`. This is a backend/driver response-parsing
failure (missing table statistics on the remote engine), not a user-input
error, so it is bucketed as `500 GENERIC_BACKEND_ERROR` (code 1011) rather than
`400 GENERIC_COMMAND_ERROR`.
This mirrors the pre-existing sibling conversion in the very same function —
the raw `jinja2.TemplateError` handling added by #42757 (`fix(sqllab): wrap
process_template() in QueryEstimationCommand to prevent raw UndefinedError
leak`) — and continues the SQL Lab exception-cleanup series (#43883 / #43795 /
#43772).
The change is strictly additive: successful estimates are unchanged; only
the previously-unhandled malformed/`NaN` cost-estimate JSON now returns a clean
typed 500.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
_N/A — backend error-handling change._
**Before:** opaque unhandled `500 GENERIC_BACKEND_ERROR` from an escaped
`simplejson.JSONDecodeError`.
**After:** typed `SupersetErrorException` → `500 GENERIC_BACKEND_ERROR` with
message _"Unable to parse the cost estimate returned by the database."_
### TESTING INSTRUCTIONS
Automated (new regression test in
`tests/unit_tests/commands/sql_lab/test_estimate.py`):
```
pytest tests/unit_tests/commands/sql_lab/test_estimate.py
```
`test_run_wraps_raw_jsondecodeerror_from_cost_estimation` mocks
`estimate_query_cost` to raise the concrete simplejson `JSONDecodeError` (via
`superset.utils.json.JSONDecodeError`, which *is*
`simplejson.errors.JSONDecodeError` — distinct from the stdlib class) and
asserts `run()` raises `SupersetErrorException` with `.status == 500` and
`.error.error_type == GENERIC_BACKEND_ERROR`. Verified failing on pre-fix code
and passing after.
Manual: in SQL Lab, point at a Presto/Trino database, select a table with no
computed statistics, enter any `SELECT`, and click **Estimate cost**.
Previously an opaque 500; now a clean typed backend error.
### 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
🤖 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]
