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

SQLite3 retains primary key after dropping column #502

Closed
sqlalchemy-bot opened this Issue Jul 11, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@sqlalchemy-bot

sqlalchemy-bot commented Jul 11, 2018

Migrated issue, originally created by Joel Tio (@joeltio)

Alembic 0.9.10, tested on SQLite 3.23.1.

Minimal Example (a revision):

def upgrade():
    op.create_table(
        "test_table",
        sa.Column("id", sa.Integer, primary_key=True),
        sa.Column("some_column", sa.String),
    )

    with op.batch_alter_table("test_table", schema=None) as batch_op:
        batch_op.drop_column("id")
        batch_op.add_column(sa.Column("id", sa.Integer))


def downgrade():
    op.drop_table("test_table")

When I .dump test_table after executing alembic upgrade head, I get the following output:

PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE IF NOT EXISTS "test_table" (
        some_column VARCHAR,
        id INTEGER NOT NULL,
        PRIMARY KEY (id)
);
COMMIT;

The primary key id is retained even after dropping the column.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 11, 2018

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 12, 2018

Joel Tio (@joeltio) wrote:

After applying the changes to my venv, it works.

For posterity, since the fix hasn't been released yet, I changed the batch.py file in venv/lib/python3.6/site-packages/alembic/operations/.
As in the diff of the link above:

@@ -334,6 +334,9 @@
         self.column_transfers[column.name] = {}
 
     def drop_column(self, table_name, column, **kw):
+        existing = self.columns[column.name]
+        if column.name in self.table.primary_key.columns:
+            self.table.primary_key.columns.remove(existing)
         del self.columns[column.name]
         del self.column_transfers[column.name]
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 12, 2018

Michael Bayer (@zzzeek) wrote:

Remove column from primary key when dropping

Fixed issue in batch where dropping a primary key column, then adding it
back under the same name but without the primary_key flag, would not remove
it from the existing PrimaryKeyConstraint. If a new PrimaryKeyConstraint
is added, it is used as-is, as was the case before.

Change-Id: Id79c793fbde1a17393adeb75c2da39f191e676e6
Fixes: #502

618f0d9

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Jul 12, 2018

Changes by Michael Bayer (@zzzeek):

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