sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.7k stars 234 forks source link

Autogeneration causes invalid SQL with some PostGRES serial columns #1504

Open adamlogan73 opened 1 month ago

adamlogan73 commented 1 month ago

Describe the bug Auto-generated PostgreSQL code in 1.13.2 contains invalid code causing migrations to fail. This appears to be around Serial columns and appears to be related to the fix implemented in: https://github.com/sqlalchemy/alembic/issues/1479. I get the log around detecting a serial and omitting it. We have a ton of tables like this, and not all of them are having this behavior. The table definitions haven't changed, and everything works in 1.13.1.

The id columns definitely have a server default set, but all of them do, not just the ones triggering this behavior. I am having trouble figuring out what is different between them. The python definition is the same, and the database definitions appear to be almost identical.

I tried replicating it in a fresh database to generate a minimum code sample, but couldn't replicate the behavior

Expected behavior Expect no change from previous behavior.

To Reproduce Table Definition:

class GEO(Base):
    __tablename__ = "GEO"
    id: Mapped[int] = mapped_column(Identity(on_null=True), primary_key=True)
    name: Mapped[str] = mapped_column(String(4), unique=True)
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision: str = 'b6001b86cfc9'
down_revision: Union[str, None] = 'd4e45e048b78'
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

def upgrade() -> None:
    op.alter_column('GEO', 'id',
               existing_type=sa.INTEGER(),
               server_default=sa.Identity(always=False, on_null=True),
               existing_nullable=False,
               autoincrement=True)

def downgrade() -> None:
    op.alter_column('GEO', 'id',
               existing_type=sa.INTEGER(),
               server_default=None,
               existing_nullable=False,
               autoincrement=True)

Error

Generated SQL it tries to run during migration is: ALTER TABLE "GEO" ALTER COLUMN id; Which doesn't do anything and isn't valid and throws an error.

Versions.

JabberWocky-22 commented 1 month ago

Sorry for the bad experience :(

Serial and identity are not exactly same in postgres. Serial column has default value while identity column not. In table definition id column use identity, but the log suggest column has default value. Maybe there is mismatch in python definition and database tables. You can check identity_generation and column_default from information_schema.columns for more infomations.

zzzeek commented 1 day ago

@CaselIT any thoughts on this?