accommodate self-referential FKs, document best practice for PRAGMA FOREIGN KEYS in batch

Issue #345 resolved
Christian Benke
created an issue

On a sqlite-DB I'm trying to alter a table with

with op.batch_alter_table('resources', schema=None) as batch_op:
        batch_op.add_column(sa.Column('parent_id', sa.BigInteger(),
                    sa.ForeignKey('resources.resource_id',
                      onupdate='CASCADE', ondelete='SET NULL')))

but the foreign key I end up with points to "_alembic_patch_temp" instead of "resources":

CONSTRAINT fk_resources_parent_id_resources FOREIGN KEY(parent_id) REFERENCES _alembic_batch_temp (resource_id) ON DELETE SET NULL ON UPDATE CASCADE,

Comments (11)

  1. Michael Bayer repo owner

    the table "_alembic_batch_temp" gets renamed to "resources" as the final step. Does that part fail ? or is sqlite so lame that it leaves the FK pointing to nothing?

  2. Christian Benke reporter

    Unfortunately that step seems to fail, the reference to "_alembic_batch_temp" is the final result. Here's the full patch I apply (Adapted from ziggurat_foundations):

    """add id/parent id to resource structure
    
    Revision ID: 5c84d7260c5
    Revises: 24ab8d11f014
    Create Date: 2011-11-11 00:09:09.624704
    
    """
    
    # downgrade revision identifier, used by Alembic.
    revision = '5c84d7260c5'
    down_revision = '24ab8d11f014'
    
    from alembic.op import *
    import sqlalchemy as sa
    
    def upgrade():
        add_column('resources', sa.Column('parent_id', sa.BigInteger(),
            sa.ForeignKey('resources.resource_id',
                          onupdate='CASCADE', ondelete='SET NULL')))
    
    def downgrade():
        pass
    
  3. Michael Bayer repo owner

    ok in your env.py, what happens if you emit "PRAGMA foreign_keys=ON" on the connection? Basically if this proceeds without any errors, then what we might do is make self-ref FKs just refer to the as-yet-nonexistent table. but this is weird and might even be considered a SQLite bug (well of course it is, but a bug that sqlite would actually care about...which is less likely).

  4. Christian Benke reporter

    Yes, that did the trick, thanks a lot! For reference how to add "PRAGMA foreign_keys=ON to env.py":

    import sqlite3
    
    from sqlalchemy.engine import Engine
    from sqlalchemy import event
    
    @event.listens_for(Engine, "connect")
    def set_sqlite_pragma(dbapi_connection, connection_record):
        if type(dbapi_connection) is sqlite3.Connection:  # play well with other DB backends
           cursor = dbapi_connection.cursor()
           cursor.execute("PRAGMA foreign_keys=ON")
           cursor.close()
    
  5. Michael Bayer repo owner

    right but funny thing, with FKs on, now batch will fail for a table that has other tables pointing to it. and that is much harder to solve.

    so here i think there's the need to:

    1. document what PRAGMA FOREIGN KEYS should be set to, and how, and it might need to be off in the general case

    2. add some behavior / flag that accommodates at least when PRAGMA FOREIGN KEYS is off and sets the FK to the original table name for self-referential

  6. Michael Bayer repo owner

    as a workaround for now, you can temporarily set PRAGMA FOREIGN KEYS in your migration script using op.execute(), then turn it off. because if you leave it on , subsequent batch migrations will likely break for tables that refer to a batch-migrated table, since we're dropping a table that they refer to.

  7. Michael Bayer repo owner

    so yeah the right way to do this is going to break it for you, it's that we generate the FK constraint pointing to the ultimate table name, which will break if you have PRAGMA FOREIGN KEYS turned on. This makes it consistent with non-self-referential FKs which will never work w PRAGMA FKS on. So that'll be 0.8.4.

  8. Michael Bayer repo owner
    • Batch mode generates a FOREIGN KEY constraint that is self-referential using the ultimate table name, rather than _alembic_batch_temp. When the table is renamed from _alembic_batch_temp back to the original name, the FK now points to the right name. This will not work if referential integrity is being enforced (eg. SQLite "PRAGMA FOREIGN_KEYS=ON") since the original table is dropped and the new table then renamed to that name, however this is now consistent with how foreign key constraints on other tables already operate with batch mode; these don't support batch mode if referential integrity is enabled in any case. fixes #345

    → <<cset da7d59941872>>

  9. Log in to comment