New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transaction-dependent migrations / "pending trigger events" error with Postgresql #201

Closed
sqlalchemy-bot opened this Issue May 2, 2014 · 17 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented May 2, 2014

Migrated issue, originally created by Jack Zhou (@univerio)

It seems by default alembic runs a sequence of migrations in a single transaction. However, this causes problems in e.g. PostgreSQL where certain queries cannot run together in the same transaction and fail with an error like:

#!

sqlalchemy.exc.OperationalError: (OperationalError) cannot ALTER TABLE "some_table" because it has pending trigger events

Because of this, upgrading a database from None to head all at once might fail while upgrading one None to <intermediate_revision> and then from <intermediate_revision> to head would succeed.

It would be helpful to have the option of running each migration in its own transaction (which means alembic would stop at the first failed migration without rolling back all the previous migrations).


Attachments: 201.patch

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Jack Zhou (@univerio):

  • edited description
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

you can do this simply by changing your env.py to not call "begin_transaction()" and also send "transactional_ddl=False" to environment.configure() so that it writes a new version entry after each migration file:

connection = engine.connect()
context.configure(
            connection=connection,
            target_metadata=target_metadata,
            transactional_ddl=False
            )

try:
    context.run_migrations()
finally:
    connection.close()
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

also i'd need some more detail on what specifically causes this condition, what "pending trigger events" means. I've used alembic with Postgresql extensively and haven't seen this nor has anyone else reported it.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Michael Bayer (@zzzeek):

  • added labels: execution model
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Michael Bayer (@zzzeek):

  • changed title from "run each individual migration in its own transacti" to "transaction-dependent migrations / "pending trigge"
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

in fact just send transactional_ddl=False, leave the rest the same; no transaction is begun by context.begin_transaction() when that flag is False.

this of course is no transaction at all but if PG doesn't allow certain migration instructions in the same trans, the two operations could be in the same migration file in any case. I'd like to understand the actual case better.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Jack Zhou (@univerio) wrote:

Thank you for the quick response. This error happens, in my case, due to a DEFERRABLE foreign key constraint on my table. The following reproduces the error:

BEGIN;
CREATE TABLE test (id serial PRIMARY KEY, self_id int REFERENCES test (id) DEFERRABLE INITIALLY DEFERRED);
INSERT INTO test (id, self_id) VALUES (1, 1);
ALTER TABLE test DROP COLUMN self_id;

Here is a relevant StackOverflow question.

I'd still like to have transactions within each migration, though, instead of no transaction at all.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

OK, while I have ideas like a directive to "restart" the transaction and such, I can still see the value in transaction per-migration overall and there's no mechanics in place to allow that. I'll see what I can do.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Michael Bayer (@zzzeek):

  • added labels: high priority
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

here's an initial patch, needs tests written. feel free to try it out / debug, this is just a 5 minute version.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Michael Bayer (@zzzeek):

  • attached file 201.patch
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Jack Zhou (@univerio) wrote:

I had to add one line to make it work (the migration context can't find the environment), but other than that it worked like a charm. Thanks again!

--- a/environment.py	2014-05-02 11:46:07.026403613 -0700
+++ b/environment.py	2014-05-02 11:48:56.906397007 -0700
@@ -679,6 +679,7 @@
             dialect_name=dialect_name,
             opts=opts
         )
+        self._migration_context.environment = self
 
     def run_migrations(self, **kw):
         """Run migrations as determined by the current command line
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

OK I'm running some tests here and it needs some more work than that actually, I'll have a full feature committed soon.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

  • Added new feature :paramref:.EnvironmentContext.configure.transaction_per_migration,
    which when True causes the BEGIN/COMMIT pair to incur for each migration
    individually, rather than for the whole series of migrations. This is
    to assist with some database directives that need to be within individual
    transactions, without the need to disable transactional DDL entirely.
    fixes #201

2d490cd

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented May 2, 2014

Michael Bayer (@zzzeek) wrote:

try out master, the approach is streamlined.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Feb 10, 2016

Scott Milliken (@smilliken) wrote:

Just wanted to thumbs-up this feature. Another reason it's important:

-- migration 1
ALTER TABLE "user" DROP COLUMN deprecated_col;
-- migration 2
CREATE INDEX ... ; -- (something that takes a long time)

Batching these into the same transaction causes the Access Exclusive lock in migration 1 to be held for the duration of migration 2, causing the "user" table to be unavailable until it's finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment