gpiccione-ship-it opened a new issue, #44241:
URL: https://github.com/apache/superset/issues/44241
### Bug description
### Summary
`get_columns_description` executes the user's SQL **twice**, back to back on
the same cursor.
The result is read from the second execution, so the first is discarded
work. Beyond the direct
cost, this means a **statement-level timeout bounds only half the
operation**.
### The code (6.1.0, `superset/connectors/sqla/utils.py`)
```python
def get_columns_description(...):
# TODO(villebro): refactor to use same code that's used by
# sql_lab.py:execute_sql_statements
...
query = database.apply_limit_to_sql(query, limit=limit)
mutated_query = database.mutate_sql_based_on_config(query)
cursor.execute(mutated_query)
db_engine_spec.execute(cursor, mutated_query, database)
result = db_engine_spec.fetch_data(cursor, limit=limit)
```
`cursor.execute(mutated_query)` and `db_engine_spec.execute(cursor,
mutated_query, database)` run
the same statement one after the other; `fetch_data` then reads the cursor
left by the second. The
first call appears to serve no purpose — the result set it produces is
replaced by the next line.
### Measured (Superset 6.1.0, tenant database MariaDB 10.6.22)
On a dataset whose SQL is a `UNION` over ~7.19M rows, with
`max_statement_time = 300` in force on
the connection:
* the column probe ran for **557 s** and completed. This is not a broken
cap: the cap is per
**statement**, and there are two;
* after wrapping the probe on our side, the same probe takes **0.171 s**.
### Why the second consequence matters more than the first
An administrator who sets `max_statement_time = 300` believes they have
capped what a dataset
author can cost the database. They have capped **one statement**; the real
ceiling is **N × cap**,
and N is decided by the code, not by them. *A cap per statement does not
limit an operation.*
### A related observation on the same path
`apply_limit_to_sql` uses `limit_method = FORCE_LIMIT` and appends the limit
**at the tail**. On a
`UNION`, `LIMIT 0` therefore costs as much as `LIMIT 1` (> 20 s in our
measurement), because the
union is materialised first. Wrapping instead of appending — `SELECT * FROM
(…) AS probe WHERE 1=0`
— costs **0.005 s** and is equivalent for reading column metadata.
We had measured `SELECT * FROM (…) t LIMIT 0` at 0.045 s and concluded the
probe was cheap. It was
not, because the code does not build that shape.
### Suggested fix
* remove the `cursor.execute(mutated_query)` line immediately before
`db_engine_spec.execute(...)`;
* for metadata probing, wrap the statement rather than appending a `LIMIT`,
so that rows can be
discarded before being materialised.
### Note
We are aware of #29885, which added the `mutate_sql_based_on_config` hook
visible above — that hook
is what allowed us to work around the cost locally. The point of this report
is that the path
itself does twice the work it needs.
### Screenshots/recordings
_No response_
### Superset version
6.1.0
### Python version
Not applicable
### Node version
Not applicable
### Browser
Not applicable
### Additional context
Data source being queried: MariaDB 10.6.22 (tenant database; the metadata
database is MySQL 8.4)
Deployment: Docker Swarm, custom image built FROM apache/superset:6.1.0
Python: 3.10.21 — what the official apache/superset:6.1.0 image ships; the
dropdown only offers
3.11 and 3.12, so I selected "Not applicable"
Customizations on this same code path, declared so the numbers can be read
correctly:
- a session-level cap is applied to tenant connections (max_statement_time =
300) through
DB_CONNECTION_MUTATOR. This is what made the double execution visible in
the first place:
the probe completed after 557 s while a 300 s per-statement cap was in
force.
- we now install a SQL_QUERY_MUTATOR that rewrites the probe statement,
turning a trailing
"LIMIT 0" into "SELECT * FROM (...) AS probe WHERE 1=0".
The 557 s figure was measured WITHOUT that mutator; the 0.171 s figure is
the same probe
WITH it. Nothing else on this path is modified.
### Checklist
- [x] I have searched Superset docs and Slack and didn't find a solution to
my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar bug
report.
- [x] I have checked Superset's logs for errors and if I found a relevant
Python stacktrace, I included it here as text in the "additional context"
section.
--
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]
