rusackas commented on code in PR #41127:
URL: https://github.com/apache/superset/pull/41127#discussion_r3567828092
##########
tests/unit_tests/sql_lab_test.py:
##########
@@ -193,6 +194,107 @@ def mock_serialize_payload(payload, use_msgpack):
)
+def test_execute_sql_statements_mutates_before_split_by_default(
+ mocker: MockerFixture, app: SupersetApp
+) -> None:
+ """
+ With the default `MUTATE_AFTER_SPLIT=False`, `execute_sql_statements`
should
+ mutate the whole, un-split query once before splitting it into individual
+ statement blocks, for engines that execute statements individually rather
+ than as one. Regression guard for issue #30169.
+ """
+ query = mocker.MagicMock()
+ query.limit = 1
+ query.database = mocker.MagicMock()
+ query.database.cache_timeout = 100
+ query.status = "RUNNING"
+ query.select_as_cta = False
+ query.database.allow_run_async = True
+ query.database.db_engine_spec.engine = "sqlite"
+ query.database.db_engine_spec.run_multiple_statements_as_one = False
+ query.database.db_engine_spec.allows_sql_comments = True
+
+ mutate_mock = mocker.patch.object(
+ query.database,
+ "mutate_sql_based_on_config",
+ side_effect=lambda sql, **kw: sql,
+ )
+
+ mocker.patch("superset.sql_lab.get_query", return_value=query)
+ mocker.patch("sys.getsizeof", return_value=10000000)
+ mocker.patch(
+ "superset.sql_lab._serialize_payload",
+ side_effect=lambda payload, use_msgpack: "serialized_payload",
+ )
+ mocker.patch("superset.sql_lab.db.session.refresh", return_value=None)
+ mocker.patch("superset.sql_lab.results_backend", return_value=True)
+
+ execute_sql_statements(
+ query_id=1,
+ rendered_query="SELECT 1; SELECT 2;",
+ return_results=True,
+ store_results=True,
+ start_time=None,
+ expand_data=False,
+ log_params={},
+ )
+
+ is_split_values = [
+ call.kwargs.get("is_split") for call in mutate_mock.call_args_list
+ ]
+ # The mutator is called once on the whole, un-split query before
splitting...
+ assert is_split_values[0] is False
+ first_call_sql = mutate_mock.call_args_list[0].args[0]
+ assert "1" in first_call_sql
+ assert "2" in first_call_sql
+ # Both statements are present in a single, un-split call.
+ assert first_call_sql.count("SELECT") == 2
+ # ...and once again per already-split statement (a no-op when
+ # `MUTATE_AFTER_SPLIT=False`, since `is_split=True` won't match the
config).
+ assert all(value is True for value in is_split_values[1:])
+ assert len(is_split_values) == 3
+
+
+def test_execute_sql_statements_raises_when_mutator_strips_all_statements(
+ mocker: MockerFixture, app: SupersetApp
+) -> None:
+ """
+ A `SQL_QUERY_MUTATOR` that strips a query down to nothing (e.g. only
+ comments/whitespace) must raise a clean error instead of silently
+ producing an empty block list.
+ """
+ query = mocker.MagicMock()
+ query.limit = 1
+ query.database = mocker.MagicMock()
+ query.database.cache_timeout = 100
+ query.status = "RUNNING"
+ query.select_as_cta = False
+ query.database.allow_run_async = True
+ query.database.db_engine_spec.engine = "sqlite"
+ query.database.db_engine_spec.run_multiple_statements_as_one = False
+ query.database.db_engine_spec.allows_sql_comments = True
+
+ mocker.patch.object(
+ query.database,
+ "mutate_sql_based_on_config",
+ side_effect=lambda sql, **kw: "-- just a comment",
+ )
+
+ mocker.patch("superset.sql_lab.get_query", return_value=query)
+ mocker.patch("superset.sql_lab.db.session.refresh", return_value=None)
+
+ with pytest.raises(SupersetErrorException):
+ execute_sql_statements(
+ query_id=1,
+ rendered_query="SELECT 1;",
+ return_results=True,
+ store_results=True,
+ start_time=None,
+ expand_data=False,
+ log_params={},
+ )
Review Comment:
Confirmed — `allow_run_async=True` without a `results_backend` mock was
raising `SupersetResultsBackendNotConfigureException` before reaching the
empty-statement guard, so the test passed for the wrong reason. Added the
missing mock to match the other tests in the file.
--
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]
