sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.76k stars 241 forks source link

autogenerate failed to detect postgresql serial #1479

Closed JabberWocky-22 closed 4 months ago

JabberWocky-22 commented 4 months ago

Describe the bug

https://github.com/sqlalchemy/alembic/issues/73 adds support for detecting postgrersql SERIAL, so alembic won't take nextval(some sequence) as server default for column of serial type in autogenerate. But it doesn't take effect with table not under search path.

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

To Reproduce Run the code below with an empty dataabse.

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

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

Error

# ### commands auto generated by Alembic - please adjust! ###
op.create_table('account',
sa.Column('id', sa.INTEGER(), server_default=sa.text("nextval('test.account_id_seq'::regclass)"), autoincrement=True, nullable=False),
sa.PrimaryKeyConstraint('id', name='account_pkey'),
schema='test'
)
# ### end Alembic commands ###

Versions.

Additional context

In the column reflect function, it tries to check the relation of sequence between table by sequence name. The seqname contains schema info, while pgclass's relname not. Maybe should cast sequence name to regclass and filter by pgclass.oid?

Have a nice day!

CaselIT commented 4 months ago

Hi,

You are likely right about the query. Thanks for reporting.

Care to create a PR for it?

JabberWocky-22 commented 4 months ago

Sure! Pelase take some time to review :)