The superset/sql/ tree is held to 100% unit-test coverage in CI; the
SupersetParseError branch of the joined-block emptiness validation was
the only uncovered path after the shared build_statement_blocks refactor.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The empty-statement guard added for split engines never covered the
run_multiple_statements_as_one branch: with MUTATE_AFTER_SPLIT=True the
per-statement mutator outputs were joined into a single block with no
check that any executable SQL remained, so an empty/comment-only mutator
result bypassed the clean INVALID_SQL_ERROR and reached execution as an
empty block. (An earlier review reply claimed this was fixed, but the
commit never landed.)
Per review feedback, the whole run_multiple_statements_as_one ×
MUTATE_AFTER_SPLIT matrix now lives in one shared helper,
build_statement_blocks() in superset/sql/execution/executor.py, used by
both the sync (sql_lab.py) and async (celery_task.py) paths, so the two
can't drift again. The new guard validates the joined block via SQLScript
and raises the same INVALID_SQL_ERROR as the split-engine branch; if the
mutator emits engine-specific SQL our parser can't handle, validation is
skipped and the database stays the authority on validity.
Regression tests cover the previously-unguarded matrix cell in both the
sync and async suites.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add explicit type annotations for the `blocks` list in sql_lab.py and
the mock/cursor test variables, and replace an untyped lambda mutator
with a typed helper to keep MUTATE_AFTER_SPLIT type-hint coverage
consistent with the rest of the PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MUTATE_AFTER_SPLIT=True never fired the mutator for engines with
run_multiple_statements_as_one=True (BigQuery, Datastore, Kusto), since
those engines always build a single joined block and the per-block
mutation call always passes is_split=False. Mutate each statement
before joining them into that block so the flag applies consistently.
Verified separately via a full `pre-commit run` (all hooks touching
these files passed: auto-walrus, mypy, ruff-format, ruff, pylint) run
outside the git hook, whose invocation via the system /usr/bin/python3
hits an unrelated pre-existing environment issue installing the zizmor
hook (requires Python >=3.10, system python3 here is older) rather
than anything in this change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add missing coverage for the MUTATE_AFTER_SPLIT=True branch, add type
hints to satisfy dev-standard lint, and guard against a SQL_QUERY_MUTATOR
that strips all statements or changes statement count when mutating
before the split.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The prior fix derived is_split purely from run_multiple_statements_as_one, so
for the common case (engines that execute statements individually) with the
default MUTATE_AFTER_SPLIT=False, is_split was always True and the mutator
never fired at all -- a regression from the pre-fix behavior where it always
ran. Mutate the whole, un-split query once before splitting into per-statement
blocks when MUTATE_AFTER_SPLIT is False, in both the sync (sql_lab.py) and
async (celery_task.py) SQL Lab paths, and add regression tests covering both
flag values across both engine modes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SQL Lab passed the default is_split=False to mutate_sql_based_on_config for
every block, so SQL_QUERY_MUTATOR fired only when MUTATE_AFTER_SPLIT=False and
never when True. Pass is_split based on whether the engine runs statements as
one block, matching the canonical usage in db_engine_specs and the new
execution engine, and thread the same flag through execute_sql_with_cursor so
the async (Celery) path is consistent. Adds unit tests for both the guard
semantics and the executor contract, plus a config note.
Closes#30169
Supersedes #34111
Co-authored-by: Lucas Wolkersdorfer <lucas.wolkersdorfer@rise-world.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
