Repository navigation
fix: run non-transactional migration statements separately - #561
Open
RitiGrover wants to merge 1 commit into
Open
RitiGrover wants to merge 1 commit into
RitiGrover wants to merge 1 commit into
Conversation
Command._upgrade ran the whole migration script through a single execute_script() call regardless of RUN_IN_TRANSACTION. Sending multiple statements in one query implicitly starts a transaction on PostgreSQL even when the connection itself isn't in one, so a migration opted out of a transaction specifically to run e.g. CREATE/DROP INDEX CONCURRENTLY would still fail with 'cannot run inside a transaction block' as soon as it had more than one such statement. When run_in_transaction is False, split the script and execute each statement on its own instead. Fixes tortoise#467
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Command._upgradealways ran the migration script through a singleconn.execute_script(script)call, regardless ofRUN_IN_TRANSACTION. Sending multiple statements in one query implicitly starts a transaction on PostgreSQL even when the connection itself isn't inside one (this isasyncpg/PostgreSQL's own simple-query-protocol behavior, not something aerich or tortoise-orm layers on top). A migration that setsRUN_IN_TRANSACTION = Falsespecifically to runCREATE/DROP INDEX CONCURRENTLYwould still fail with... cannot run inside a transaction blockas soon as it had more than one such statement, since the implicit transaction wrapping happens regardless of the Python-level transaction state.Motivation and Context
Fixes #467. Confirmed directly against a real PostgreSQL 16 instance: sending two
CREATE INDEX CONCURRENTLYstatements in oneexecute()call fails with exactly this error, while sending the identical two statements as separate calls succeeds. This matches the reporter's own observation in the thread ("if there is a single CONCURRENTLY statement it works ... if there is more than 1 it does not").How Has This Been Tested?
asyncpgdirectly, isolating it to the multi-statement-in-one-call behavior (unrelated to any Python-level transaction).split_sql_statements(aerich/utils.py) and used it inCommand._upgradeonly for the non-transactional path, so the existing transactional path (execute_scriptwith the full multi-statement script) is untouched.tests/test_run_in_transaction.py:test_split_sql_statements- unit test for the split helper.test_upgrade_runs_concurrently_statements_outside_transaction(Postgres-only, viarequires_dialect("postgres")) - builds a real migration file with twoCREATE INDEX CONCURRENTLYstatements andRUN_IN_TRANSACTION = False, runs it throughCommand._upgrade, and asserts both indexes exist.execute_scriptbehavior reproduces the exact sameTransactionManagementError: CREATE INDEX CONCURRENTLY cannot run inside a transaction blockfrom the issue; the fix resolves it.tests/test_command.py,tests/test_utils.py) against SQLite - no regressions (one pre-existing failure intest_read_config_from_class_varis an unrelated missingpydanticdependency in my local env, not caused by this change).ruff check,ruff format --check, andmypyall clean on the changed files.