Autogeneration causes invalid SQL with some PostGRES serial columns

[adamlogan73]

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: #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.

• OS: Docker in docker container
• Python: 3.11
• Alembic: 1.13.2
• SQLAlchemy: 2.0.31
• Database: RDS PostGRES 14.10
• DBAPI: psycopg2

[zzzeek]

@CaselIT any thoughts on this?

[CaselIT]

seems like a regression due to the mentioned issue.
we likely need to better distinguish serial vs identity.

[RitiGrover]

I dug into this and confirmed the root cause: Operations.alter_column defaults existing_server_default to False (its "not specified" sentinel), but visit_identity_column in alembic/ddl/postgresql.py only checks is None to decide whether to render ADD GENERATED ... AS IDENTITY. False falls through into the "diff against an existing identity" branch, computes an empty diff against the sentinel, and renders an invalid, clause-less ALTER TABLE ... ALTER COLUMN id;.

Reproduced against a real PostgreSQL 16 instance with the table definition from this issue and confirmed the exact invalid SQL. The fix is a one-line change (treat False the same as None in that check), verified against Postgres that it also doesn't break the other two identity paths (altering existing identity options, dropping identity), and comes with a regression test.

I opened #1869 with this, but it looks like it needs the issue labeled "open for pull requests" first. Happy to reopen it once that's done, or take feedback if the approach should be different.

[sqla-tester]

Riti Grover has proposed a fix for this issue in the main branch:

fix(postgresql): identity column not added when existing_server_default omitted https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6982

[zzzeek]

sure, it's fine

[sqla-tester]

Riti Grover has proposed a fix for this issue in the main branch:

fix(postgresql): identity column not added when existing_server_default omitted https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/6982