sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.85k stars 247 forks source link

batch_op removes sqlite_autoincrement #380

Open sqlalchemy-bot opened 8 years ago

sqlalchemy-bot commented 8 years ago

Migrated issue, originally created by berend (@berend)

Given a sqlite3 table with __tableargs__ = {'sqlite_autoincrement': True} and a batch_op migration touching that table:

Model. initial migration

def upgrade():
    op.create_table('person',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('firstname', sa.String(), nullable=False),
    sa.Column('lastname', sa.String(), nullable=False),
    sa.PrimaryKeyConstraint('id'),
    sqlite_autoincrement=True
    )

simple migration, making firstname nullable:

def upgrade():
    with op.batch_alter_table('person') as batch_op:
        batch_op.alter_column(
            'firstname',
            existing_type=sa.TEXT(),
            server_default=None,
            nullable=True)

With echo-sql on I can see, that the first version of the person table has autoincrement on the id column. During the second migration, alembic creates a temp table, because sqlite does not support alter column operations. That temp table does not have the autoincrement on the id column. After renaming it to person, the autoincrement is gone

Manually adding table kwargs to the batch_op is a workaround.

def upgrade():
    with op.batch_alter_table('person', table_kwargs={'sqlite_autoincrement': True}) as batch_op:
        batch_op.alter_column(
            'firstname',
            existing_type=sa.TEXT(),
            server_default=None,
            nullable=True)

Do I have to always use table_kwargs to let alembic know? Or is there a bug when reading the table to create the temp table?

sqlalchemy-bot commented 8 years ago

Michael Bayer (@zzzeek) wrote:

well SQLAlchemy would have to reflect this which I don't believe it does right now. SQLA can accept a pull request to include reflection of this option. sqlite_autoincrement is a rarely used option so the workaround here is not that unreasonable.

sqlalchemy-bot commented 8 years ago

Changes by berend (@berend):

sqlalchemy-bot commented 8 years ago

Changes by berend (@berend):

AbdealiLoKo commented 1 year ago

Should the autogeneration option in alembic autogenerate the table_kwargs={'sqlite_autoincrement': True} ?

zzzeek commented 1 year ago

oh this is batch, not autogenerate.

zzzeek commented 1 year ago

Should the autogeneration option in alembic autogenerate the table_kwargs={'sqlite_autoincrement': True} ?

this issue regards batch mode for SQLite. i dont understand the question

AbdealiLoKo commented 1 year ago

I was trying to use sqlite_autoincrement and I realized I had this same issue.

Where when I create a new ORM model:

class ExampleTable(WithoutWorkspace, BaseModel):
    __tablename__ = "example_table"
    __tableargs__ = {"sqlite_autoincrement": True}

    id = Column(BigInteger, primary_key=True)

And then I use alembic's alembic migrate - it creates:

def upgrade():
    op.create_table(
        "example_table",
        sa.Column("id", sa.BigInteger(), nullable=False),
        sa.PrimaryKeyConstraint("id", name=op.f("pk_example_table")),
        sqlite_autoincrement=True,
    )

Now, I use context.configure(..., render_as_batch=True) in my env.py So, when I make a change to a column (let's say I add a column to this table). The generated code is:

def upgrade():
    with op.batch_alter_table("example_table", schema=None) as batch_op:
        batch_op.add_column(sa.Column("name", sa.String(), nullable=True))

if the recommendation here is to use: with op.batch_alter_table("example_table", schema=None, table_kwargs={'sqlite_autoincrement': True}) as batch_op I am wondering if alembic should have generate that for me

zzzeek commented 1 year ago

batch op when it drops and recreates the table would ideally know about sqlite_autoincrement, however right now it doesn't , so autogenerate should not generate this since the above is a workaround only that would in theory eventually be fixed in batch mode itself.