aminghadersohi commented on code in PR #45232:
URL: https://github.com/apache/superset/pull/45232#discussion_r4278969897
##########
superset/sql/execution/celery_task.py:
##########
@@ -481,25 +544,44 @@ def _execute_sql_statements(
)
) from ex
- # Check if stopped
- if not execution_results:
- payload.update({"status": QueryStatus.STOPPED.value})
- return payload
+ # Cancelling can clear cursor.description, leaving a nonempty result
+ # list that looks like successful DML. Check intent, not just results.
+ stopped = not execution_results or check_stopped()
# Commit for mutations
- if parsed_script.has_mutation() or query.select_as_cta:
+ if not stopped and (parsed_script.has_mutation() or
query.select_as_cta):
conn.commit() # pylint: disable=consider-using-transaction
Review Comment:
Correction: my earlier replies in this thread are wrong for this PR. When I
split it, the all-engine stop/commit finalization work (the `on_commit`
callback, the `durable=True` error handling and the legacy
`superset/sql_lab.py` changes) moved to
https://github.com/apache/superset/pull/45283 . At this PR's head
758e87f60fa44f24e8d7297aa61a4cfb3d4bf28c that code is not present, so this PR
keeps the Impala routing change and the finished-status checks it needs.
Remaining: a stop that loses the race to a committed mutation or CTAS can
still end as STOPPED on the Celery and legacy SQL Lab paths (success and
post-commit failure). That work is in
https://github.com/apache/superset/pull/45283 , not here.
--
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]
