aminghadersohi commented on PR #45283: URL: https://github.com/apache/superset/pull/45283#issuecomment-6191946155
Celery autocommit INSERT/CTAS could overwrite an accepted Stop with SUCCESS and publish results when engine cancellation was unreachable or no cancellation ID was available. Commit 586cbd6cf2d2be9f4f32325d38c130513467836c fixes this by re-reading Stop after execution and guarding success, error, and results-backend finalization in both worker paths. It also records Celery dispatch without a cancel ID, retains cancellation exception details, and corrects the documented 422 response. An accepted Stop leaves the query STOPPED and discards its results, but an autocommit mutation can still complete on the engine. The SQL Lab docs and UPDATING.md explicitly distinguish Stop from rollback. Regression tests, with production changes reverted once and then restored: - `test_accepted_stop_with_real_postgres_workers`: the autocommit INSERT/CTAS matrix covers API and async-handle callers, both worker paths, and normal/unreachable/no-ID cancellation. Before: 8 failed, 16 passed (the eight Celery unavailable-cancellation cases ended SUCCESS). After: 24 passed. The tests assert STOPPED in the row, payload and UI query representation, no results key/backend publication, and the actual database mutation outcome. - `test_stop_during_mutation_commit_discards_results`, `test_failure_after_stop_raced_commit_preserves_stopped`, `test_get_sql_results_preserves_stop_when_mutation_commits`, and `test_celery_records_dispatch_before_execution_without_cancel_handle` are included in the focused unit regression selection: before 46 failed/57 passed; after 103 passed. Validation: - `pytest tests/unit_tests/sql_lab_test.py tests/unit_tests/sql/ tests/unit_tests/daos/`: 2704 passed. - `pytest tests/testcontainers/db_engine_specs/test_postgres.py -m testcontainers`: 82 passed on Postgres 17. - `pytest tests/integration_tests/queries/api_tests.py -k stop_query`: 3 passed. - `npm run test -- src/SqlLab/actions/sqlLab.test.ts --testNamePattern='stopQuery|postStopQuery|stops query' --silent`: 7 passed. - `pre-commit run`: all applicable hooks passed on this commit's staged files, including mypy. A separate run across all branch-changed files failed frontend type-checking with TS6305 for unbuilt workspace declaration outputs; its other hooks passed. Siblings checked: - superset/sql/execution/celery_task.py — fixed in 586cbd6cf2d2be9f4f32325d38c130513467836c - superset/sql_lab.py — fixed in 586cbd6cf2d2be9f4f32325d38c130513467836c - superset/daos/query.py — fixed in 586cbd6cf2d2be9f4f32325d38c130513467836c - superset/sql/execution/executor.py — fixed in 586cbd6cf2d2be9f4f32325d38c130513467836c - superset-frontend/src/SqlLab/actions/sqlLab.ts — not affected — successful Stop already dispatches STOP_QUERY and completed-query 422 responses refresh the terminal state. -- 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]
