postgresql serial column detection might fail

[JabberWocky-22]

Describe the bug
There is a probability that serial column was not detected.
This can happen when the table is referenced by another table.

Expected behavior
The result of autogererate should't contain server default for column id.

To Reproduce

import sqlalchemy as sa
from alembic.autogenerate.api import produce_migrations
from alembic.autogenerate.api import render_python_code
from alembic.migration import MigrationContext


def filter_by_schema(schema_name: str):
    def _filter_by_schema(name, type_, parent_names):
        if type_ == "schema":
            return name == schema_name
        else:
            return True

    return _filter_by_schema


engine = sa.create_engine("postgresql://name:pass@host:1234/tmp")
with engine.connect() as conn, conn.begin():
    conn.execute(
        sa.text(
            """
        DROP SCHEMA IF EXISTS test CASCADE;
        CREATE SCHEMA test;
        CREATE TABLE test.account1(id SERIAL PRIMARY KEY);
        CREATE TABLE test.account2(id SERIAL PRIMARY KEY, id2 int REFERENCES test.account1(id));
        """
        )
    )
metadata = sa.MetaData(schema="test")
mc = MigrationContext.configure(
    connection=engine.connect(),
    opts={"include_name": filter_by_schema("test"), "include_schemas": True},
)
migration_script = produce_migrations(mc, metadata)
downgrade_code = render_python_code(migration_script.downgrade_ops)
print(downgrade_code)

Error
It will produce two outputs and the first one is incorrect.

# ### commands auto generated by Alembic - please adjust! ###
    op.create_table('account1',
    sa.Column('id', sa.INTEGER(), server_default=sa.text("nextval('test.account1_id_seq'::regclass)"), autoincrement=True, nullable=False),
    sa.PrimaryKeyConstraint('id', name='account1_pkey'),
    schema='test',
    postgresql_ignore_search_path=False
    )
    op.create_table('account2',
    sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False),
    sa.Column('id2', sa.INTEGER(), autoincrement=False, nullable=True),
    sa.ForeignKeyConstraint(['id2'], ['test.account1.id'], name='account2_id2_fkey'),
    sa.PrimaryKeyConstraint('id', name='account2_pkey'),
    schema='test'
    )
    # ### end Alembic commands ###

# ### commands auto generated by Alembic - please adjust! ###
    op.create_table('account2',
    sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False),
    sa.Column('id2', sa.INTEGER(), autoincrement=False, nullable=True),
    sa.ForeignKeyConstraint(['id2'], ['test.account1.id'], name='account2_id2_fkey'),
    sa.PrimaryKeyConstraint('id', name='account2_pkey'),
    schema='test'
    )
    op.create_table('account1',
    sa.Column('id', sa.INTEGER(), autoincrement=True, nullable=False),
    sa.PrimaryKeyConstraint('id', name='account1_pkey'),
    schema='test'
    )
    # ### end Alembic commands ###

Versions.

• OS: macos 14.6.1
• Python: 3.10.9
• Alembic: 1.13.3
• SQLAlchemy: 2.0.36
• Database: PostgreSQL 15.7
• DBAPI: psycopg2-binary 2.9.10

Additional context
If table is not fererenced by other table, the result is always correct.
If I make both tables reference each other, the first table in autogenerate is alway incorrect.

ALTER TABLE test.account1 ADD FOREIGN KEY (id) REFERENCES test.account2(id);

I guess the listen hook migth fail with foreign keys, not sure since havn't dig into code much.

Have a nice day!

[CaselIT]

Hi,

It seems like a bug, but I'm proposing to close as won't fix.

The reason is that they table definition that you have are highly unlikely, since it doesn't make much sense adding a fk on an auto-incrementing column.
Since postgresql does not tell you if a column is a serial when reflecting a table, alembic has some heuristic to figure out when it is and whet it isn't. The presence of a fk on the column likely makes alembic think that the column is not a serial.

Thoughts @zzzeek ?

[JabberWocky-22]

Actually, it can happen on a table with serial column and being referenced by another table(doesn't matter adding foreign key on which column), just like:

CREATE SCHEMA test;
CREATE TABLE test.account1(short_id SERIAL PRIMARY KEY, id uuid unique);
CREATE TABLE test.account2(short_id SERIAL PRIMARY KEY, id uuid unique REFERENCES test.account1(id));
ALTER TABLE test.account1 ADD FOREIGN KEY (id) REFERENCES test.account2(id);  -- not necessary

Besides two tables reference each other just make it easier to reproduce, it's not necessary.

[JabberWocky-22]

alembic/alembic/autogenerate/compare.py

Lines 204 to 219 in 431fd07

	for s, tname in conn_table_names.difference(metadata_table_names):
	name = sa_schema._get_table_key(tname, s)
	exists = name in removal_metadata.tables
	t = sa_schema.Table(tname, removal_metadata, schema=s)
	if not exists:
	event.listen(
	t,
	"column_reflect",
	# fmt: off
	autogen_context.migration_context.impl.
	_compat_autogen_column_reflect
	(inspector),
	# fmt: on
	)
	sqla_compat._reflect_table(inspector, t)

Looks like when reflecting table, the referenced table will be reflected too, since the default value of resolve_fks is True.
And the event hook won't take effect for the referenced table.

another similar issue: #1454