rusackas opened a new pull request, #41127:
URL: https://github.com/apache/superset/pull/41127
### SUMMARY
In SQL Lab, every statement block was passed to
`Database.mutate_sql_based_on_config()` with the default `is_split=False`. That
method only invokes `SQL_QUERY_MUTATOR` when `is_split == MUTATE_AFTER_SPLIT`,
so the mutator fired **only** when `MUTATE_AFTER_SPLIT=False` and **never**
when `MUTATE_AFTER_SPLIT=True` — meaning operators who rely on the mutator for
per-statement governance (e.g. injecting `SET ROLE <user>` before each query)
got no mutation in SQL Lab at all.
This passes `is_split` based on whether the engine spec runs all statements
as one block:
```python
is_split=not db_engine_spec.run_multiple_statements_as_one
```
- multi-statement (split) blocks → `is_split=True` → mutator fires when
`MUTATE_AFTER_SPLIT=True`
- a single merged block (`run_multiple_statements_as_one`) →
`is_split=False` → mutator fires when `MUTATE_AFTER_SPLIT=False`
This matches the canonical usage already in `db_engine_specs/base.py` and
the new execution engine (`sql/execution/executor.py`), and crucially preserves
behavior for `MUTATE_AFTER_SPLIT=False` users (no per-block over-mutation).
It also threads the same `is_split` flag through `execute_sql_with_cursor()`
so the async (Celery) path — which passes a possibly-merged block — is
consistent with the sync path, and adds a clarifying note to the
`MUTATE_AFTER_SPLIT` config comment.
This supersedes #34111, which addressed the same bug by passing
`is_split=config["MUTATE_AFTER_SPLIT"]`. That made the `is_split ==
MUTATE_AFTER_SPLIT` guard always true, firing the mutator unconditionally and
changing behavior for `MUTATE_AFTER_SPLIT=False` users with multi-statement
queries, and shipped without tests. Full credit to @LucasWolke for the original
diagnosis and fix, and to @giacomochiarella for detailing the data-governance
use case.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend behavior change.
### TESTING INSTRUCTIONS
Set in `superset_config.py`:
```python
def sql_query_mutator(sql, **kwargs):
return f"-- mutated\n{sql}"
SQL_QUERY_MUTATOR = sql_query_mutator
MUTATE_AFTER_SPLIT = True
```
Run a query in SQL Lab and confirm `executed_sql` now contains the mutation
(it previously did not with `MUTATE_AFTER_SPLIT=True`). Set
`MUTATE_AFTER_SPLIT=False` and confirm the mutator still applies to the
(un-split) query.
Automated:
```
pytest
tests/unit_tests/models/core_test.py::test_mutate_sql_based_on_config_respects_is_split
pytest
tests/unit_tests/sql/execution/test_executor.py::test_execute_sql_with_cursor_forwards_is_split
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: Closes #30169
- [ ] 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]
