rusackas commented on code in PR #41127:
URL: https://github.com/apache/superset/pull/41127#discussion_r3581894082
##########
superset/sql/execution/celery_task.py:
##########
@@ -137,8 +141,52 @@ def _prepare_statement_blocks(
# Build statement blocks for execution
if db_engine_spec.run_multiple_statements_as_one:
- blocks =
[parsed_script.format(comments=db_engine_spec.allows_sql_comments)]
+ if app.config["MUTATE_AFTER_SPLIT"]:
+ # These engines never actually execute statements individually, so
the
+ # per-block mutation call in `execute_sql_with_cursor` (whose
`is_split`
+ # is always `False` here) would never fire. Mutate each statement
here,
+ # before joining them into the single block this engine requires,
so
+ # `MUTATE_AFTER_SPLIT=True` still applies the mutator per
statement.
+ blocks = [
+ ";\n".join(
+ database.mutate_sql_based_on_config(
+
statement.format(comments=db_engine_spec.allows_sql_comments),
+ is_split=True,
+ )
+ for statement in parsed_script.statements
+ )
+ ]
Review Comment:
Good catch, added the same empty-statement guard to the
`run_multiple_statements_as_one` branch, with a regression test covering it.
##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -663,6 +663,9 @@ def test_execute_sql_statements(
mock_cursor
)
mock_query.database.db_engine_spec.run_multiple_statements_as_one =
False
+ mock_query.database.mutate_sql_based_on_config.side_effect = (
+ lambda sql_, **kwargs: sql_
+ )
Review Comment:
Lambdas can't take type annotations in Python, and this
passthrough-as-side_effect pattern is already all over the test suite
(viz_tests.py, security_tests.py, etc), so I'll leave it as is.
##########
tests/integration_tests/sqllab_tests.py:
##########
@@ -763,6 +766,9 @@ def test_execute_sql_statements_ctas(
mock_cursor
)
mock_query.database.db_engine_spec.run_multiple_statements_as_one =
False
+ mock_query.database.mutate_sql_based_on_config.side_effect = (
+ lambda sql_, **kwargs: sql_
+ )
Review Comment:
Same as the other thread, lambdas can't be type-annotated and this mirrors
the existing side_effect pattern used elsewhere in the test suite.
--
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]
