fix(db): unblock PostgreSQL upgrade to v0.7.x (sqlalchemy<2.1 + autocommit_block migrations) - #1904
Merged
Merged
Conversation
…mmit_block Fixes the v0.6.2 -> v0.7.x PostgreSQL upgrade path reported in #1902, which failed in two ways: 1. Missing psycopg DBAPI. We ship only psycopg2-binary, but `sqlalchemy>=2.0.44` allowed SQLAlchemy 2.1, which changed the default `postgresql://` driver from psycopg2 to psycopg (v3). A bare PyPI install then failed migrations with "No module named 'psycopg'". Cap to `>=2.0.44,<2.1` so psycopg2 stays the default driver (the tested/locked line) until psycopg3 is adopted. 2. CONCURRENTLY inside a transaction block. Seven migrations escaped Alembic's migration transaction with the hand-rolled `op.execute("COMMIT")` trick. That happens to work on psycopg2 but breaks on psycopg/SQLAlchemy 2.1, where the next statement re-opens a transaction and PostgreSQL rejects CREATE/DROP INDEX CONCURRENTLY. Convert all seven to `op.get_context().autocommit_block()`, matching the existing b8c9d0e1f2a3 migration. The e9b2c7d1f3a4 entity-link cleanup's `DO $$ ... COMMIT ... $$` batch loop is wrapped too, since procedural COMMIT also requires autocommit. Add two lint-style guard tests in test_migration_shape.py so this class of bug can't be reintroduced: one bans `op.execute("COMMIT")`, the other requires any migration running CONCURRENTLY DDL to open an autocommit_block().
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.
Summary
Fixes the
v0.6.2→v0.7.xPostgreSQL upgrade path reported in #1902, which failed during startup migrations in two sequential ways.1. Missing
psycopgDBAPIWe ship only
psycopg2-binary, but the dependency pin wassqlalchemy>=2.0.44with no upper bound, so a fresh PyPI install resolves SQLAlchemy 2.1. SQLAlchemy 2.1 changed the defaultpostgresql://driver from psycopg2 to psycopg (v3). Sinceto_libpq_url()produces a driver-lesspostgresql://URL, the migration engine then tried to import psycopg3 and failed withModuleNotFoundError: No module named 'psycopg'.Fix: cap
sqlalchemy>=2.0.44,<2.1so psycopg2 stays the default driver — the line that's locked (2.0.44) and tested in CI. (The repo's ownuv.lockwas already on 2.0.44, which is why CI never caught this — only fresh PyPI installs hit it.)2.
CREATE/DROP INDEX CONCURRENTLYinside a transaction blockSeven migrations escaped Alembic's migration transaction with a hand-rolled
op.execute("COMMIT")trick. That happens to work on psycopg2 but breaks on psycopg/SQLAlchemy 2.1 (the env you land in after working around #1), where the next statement re-opens a transaction and PostgreSQL rejects theCONCURRENTLYDDL withActiveSqlTransaction.Fix: convert all seven to
with op.get_context().autocommit_block():, matching the project's own newerb8c9d0e1f2a3migration. Thee9b2c7d1f3a4entity-link cleanup'sDO $$ … COMMIT … $$batch loop is wrapped too, since proceduralCOMMITalso requires autocommit.Migrations converted:
a2b3c4d5e6f8,b3c4d5e6f7g8,c1a2b3d4e5f6,d2e3f4a5b6c7,d4e5f6g7h8i9,e1b2c3d4f5a6,e9b2c7d1f3a4Regression guard
Two lint-style tests in
test_migration_shape.pyrun over every migration so this class of bug can't return:test_migration_uses_autocommit_block_not_manual_commit— bansop.execute("COMMIT").test_migration_concurrently_ddl_runs_in_autocommit_block— any migration runningCONCURRENTLYDDL must open anautocommit_block().Testing
pytest tests/test_migration_shape.py→ 140 passed, 70 skipped (the new guards).pytest tests/test_admin_backup_restore.py→ 6 passed — this builds a full schema by running the entire migration chain (including the convertedCONCURRENTLYmigrations) on real pg0, exercising theautocommit_blockpath end-to-end../scripts/hooks/lint.shclean.Notes
<2.1cap is a deliberate stopgap. Adopting SQLAlchemy 2.1 later just means addingpsycopg[binary](or pinningpostgresql+psycopg2://into_libpq_url) and lifting the ceiling — a comment inpyproject.tomlflags this.autocommit_blockis the documented-correct approach for both drivers.e9b2c7d1f3a4chunks the delete at 50k rows; no change needed.Closes #1902