mirror of
https://github.com/apache/superset.git
synced 2026-07-17 12:15:40 +00:00
fix(sqllab): mutate the whole query before splitting when MUTATE_AFTER_SPLIT=False
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>
This commit is contained in:
@@ -282,9 +282,12 @@ def test_prepare_statement_blocks_single_statement(
|
||||
"""Test statement block preparation for single statement."""
|
||||
from superset.sql.execution.celery_task import _prepare_statement_blocks
|
||||
|
||||
mock_database.mutate_sql_based_on_config = lambda sql, **kw: sql
|
||||
sql = "SELECT * FROM users"
|
||||
|
||||
script, blocks = _prepare_statement_blocks(sql, mock_database.db_engine_spec)
|
||||
script, blocks = _prepare_statement_blocks(
|
||||
sql, mock_database.db_engine_spec, mock_database
|
||||
)
|
||||
|
||||
assert len(blocks) == 1
|
||||
|
||||
@@ -295,9 +298,12 @@ def test_prepare_statement_blocks_multiple_statements(
|
||||
"""Test statement block preparation for multiple statements."""
|
||||
from superset.sql.execution.celery_task import _prepare_statement_blocks
|
||||
|
||||
mock_database.mutate_sql_based_on_config = lambda sql, **kw: sql
|
||||
sql = "SELECT * FROM users; SELECT * FROM orders;"
|
||||
|
||||
script, blocks = _prepare_statement_blocks(sql, mock_database.db_engine_spec)
|
||||
script, blocks = _prepare_statement_blocks(
|
||||
sql, mock_database.db_engine_spec, mock_database
|
||||
)
|
||||
|
||||
assert len(blocks) == 2
|
||||
|
||||
@@ -309,13 +315,43 @@ def test_prepare_statement_blocks_run_as_one(
|
||||
from superset.sql.execution.celery_task import _prepare_statement_blocks
|
||||
|
||||
mock_database.db_engine_spec.run_multiple_statements_as_one = True
|
||||
mock_database.mutate_sql_based_on_config = lambda sql, **kw: sql
|
||||
sql = "SELECT * FROM users; SELECT * FROM orders;"
|
||||
|
||||
script, blocks = _prepare_statement_blocks(sql, mock_database.db_engine_spec)
|
||||
script, blocks = _prepare_statement_blocks(
|
||||
sql, mock_database.db_engine_spec, mock_database
|
||||
)
|
||||
|
||||
assert len(blocks) == 1
|
||||
|
||||
|
||||
def test_prepare_statement_blocks_mutates_before_split_when_configured(
|
||||
app_context: None, mock_database: MagicMock, mocker: MockerFixture
|
||||
) -> None:
|
||||
"""
|
||||
`MUTATE_AFTER_SPLIT=False` should mutate the whole, un-split query before
|
||||
it gets broken into per-statement blocks, for engines that execute
|
||||
statements individually. Regression guard for issue #30169.
|
||||
"""
|
||||
from superset.sql.execution.celery_task import _prepare_statement_blocks
|
||||
|
||||
mocker.patch.dict(current_app.config, {"MUTATE_AFTER_SPLIT": False})
|
||||
mutate_mock = mocker.patch.object(
|
||||
mock_database,
|
||||
"mutate_sql_based_on_config",
|
||||
side_effect=lambda sql, **kw: f"-- mutated\n{sql}",
|
||||
)
|
||||
sql = "SELECT * FROM users; SELECT * FROM orders;"
|
||||
|
||||
_, blocks = _prepare_statement_blocks(
|
||||
sql, mock_database.db_engine_spec, mock_database
|
||||
)
|
||||
|
||||
assert len(blocks) == 2
|
||||
assert "mutated" in blocks[0]
|
||||
mutate_mock.assert_called_once_with(mocker.ANY, is_split=False)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Result Finalization Tests
|
||||
# =============================================================================
|
||||
|
||||
Reference in New Issue
Block a user