LucasWolke commented on code in PR #34111:
URL: https://github.com/apache/superset/pull/34111#discussion_r2200985443
##########
superset/sql_lab.py:
##########
@@ -475,7 +475,9 @@ def execute_sql_statements( # noqa: C901
db.session.commit()
# Hook to allow environment-specific mutation (usually comments)
to the SQL
- query.executed_sql = database.mutate_sql_based_on_config(block)
+ query.executed_sql = database.mutate_sql_based_on_config(
+ block, is_split=config["MUTATE_AFTER_SPLIT"]
Review Comment:
Thanks for your assessment. To clarify my approach: the `MUTATE_AFTER_SPLIT`
flag was introduced specifically to control when the query mutator is applied
for chart queries, as discussed in #21645. This behavior was already
established in SQL Lab, so the flag was not originally intended to affect SQL
Lab queries.
From my understanding, the `is_split` parameter is also primarily meant for
chart queries, to ensure backward compatibility.
It would be possible to create a separate `mutate_sql` function that always
applies the mutator, just for SQL Lab (if semantic clarity is the issue with my
fix), or to add a comment clarifying why the config flag is passed. However, I
believe always applying the mutator after splitting the query in SQL Lab is the
intended and correct approach - the "before or after splitting" logic was only
ever meant for chart queries, not for SQL Lab.
--
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]
