Skip to content

fix: run non-transactional migration statements separately - #561

Open
RitiGrover wants to merge 1 commit into
tortoise:devfrom
RitiGrover:fix/run-in-transaction-multi-statement-concurrently
Open

RitiGrover wants to merge 1 commit into
tortoise:devfrom
RitiGrover:fix/run-in-transaction-multi-statement-concurrently

Conversation

@RitiGrover

Copy link
Copy Markdown

Description

Command._upgrade always ran the migration script through a single conn.execute_script(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 inside one (this is asyncpg/PostgreSQL's own simple-query-protocol behavior, not something aerich or tortoise-orm layers on top). A migration that sets RUN_IN_TRANSACTION = False specifically to run CREATE/DROP INDEX CONCURRENTLY would still fail with ... cannot run inside a transaction block as 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 CONCURRENTLY statements in one execute() 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?

  • Reproduced the exact error against a live PostgreSQL 16 container using asyncpg directly, isolating it to the multi-statement-in-one-call behavior (unrelated to any Python-level transaction).
  • Added split_sql_statements (aerich/utils.py) and used it in Command._upgrade only for the non-transactional path, so the existing transactional path (execute_script with the full multi-statement script) is untouched.
  • Added 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, via requires_dialect("postgres")) - builds a real migration file with two CREATE INDEX CONCURRENTLY statements and RUN_IN_TRANSACTION = False, runs it through Command._upgrade, and asserts both indexes exist.
    • Confirmed red -> green: temporarily forcing the old single-execute_script behavior reproduces the exact same TransactionManagementError: CREATE INDEX CONCURRENTLY cannot run inside a transaction block from the issue; the fix resolves it.
  • Ran the broader suite locally (tests/test_command.py, tests/test_utils.py) against SQLite - no regressions (one pre-existing failure in test_read_config_from_class_var is an unrelated missing pydantic dependency in my local env, not caused by this change).
  • ruff check, ruff format --check, and mypy all clean on the changed files.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DROP INDEX CONCURRENTLY cannot run inside a transaction block

1 participant