Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating a self-referential foreign key with composite primary key throws a SA DuplicateColumnError #1215

Closed
Kalmis opened this issue Apr 6, 2023 · 6 comments
Labels
bug Something isn't working op directives

Comments

@Kalmis
Copy link

Kalmis commented Apr 6, 2023

Describe the bug
With SA 2, creating a self-referential foreign key with composite primary key throws a SQLAlchemy DuplicateColumnError. In more general terms, an error is thrown if self-referential foreign key has at least one same column as "source" and "target" column.

Expected behavior
The foreign key is created without an error. Works with SA 1.4, although, a deprecation warning is raised.

SADeprecationWarning: A column with name 'group_id' is already present in table 'product_mcve'. Please use method :meth:`_schema.Table.append_column` with the parameter ``replace_existing=True`` to replace an existing column.

To Reproduce

Create a migration with the code below and run it.

def upgrade():
    op.create_table(
        "product_mcve",
        sa.Column("id", postgresql.UUID(as_uuid=True), nullable=False),
        sa.Column("group_id", postgresql.UUID(as_uuid=True), nullable=False),
        sa.Column("parent_id", postgresql.UUID(as_uuid=True), nullable=False),
        sa.PrimaryKeyConstraint("id", "group_id"),
    )
    op.create_foreign_key(
        None,
        "product_mcve",
        "product_mcve",
        ["parent_id", "group_id"],
        ["id", "group_id"],
        ondelete="CASCADE",
    )

Error

Stacktrace when running migration with custom Pytest setup that resets DB & runs migrations.

self = Column('group_id', NullType(), table=None)
parent = Table('product_mcve', MetaData(), Column('parent_id', NullType(), table=<product_mcve>), Column('group_id', NullType(), table=<product_mcve>), Column('id', NullType(), table=<product_mcve>), schema=None)
all_names = {'group_id': Column('group_id', NullType(), table=<product_mcve>), 'id': Column('id', NullType(), table=<product_mcve>), 'parent_id': Column('parent_id', NullType(), table=<product_mcve>)}
allow_replacements = None, kw = {}, existing = None, extra_remove = None
existing_col = Column('group_id', NullType(), table=<product_mcve>)
conflicts_on = 'name'

    def _set_parent(  # type: ignore[override]
        self,
        parent: SchemaEventTarget,
        *,
        all_names: Dict[str, Column[Any]],
        allow_replacements: bool,
        **kw: Any,
    ) -> None:
        table = parent
        assert isinstance(table, Table)
        if not self.name:
            raise exc.ArgumentError(
                "Column must be constructed with a non-blank name or "
                "assign a non-blank .name before adding to a Table."
            )
    
        self._reset_memoizations()
    
        if self.key is None:
            self.key = self.name
    
        existing = getattr(self, "table", None)
        if existing is not None and existing is not table:
            raise exc.ArgumentError(
                "Column object '%s' already assigned to Table '%s'"
                % (self.key, existing.description)
            )
    
        extra_remove = None
        existing_col = None
        conflicts_on = ""
    
        if self.key in table._columns:
            existing_col = table._columns[self.key]
            if self.key == self.name:
                conflicts_on = "name"
            else:
                conflicts_on = "key"
        elif self.name in all_names:
            existing_col = all_names[self.name]
            extra_remove = {existing_col}
            conflicts_on = "name"
    
        if existing_col is not None:
            if existing_col is not self:
                if not allow_replacements:
>                   raise exc.DuplicateColumnError(
                        f"A column with {conflicts_on} "
                        f"""'{
                            self.key if conflicts_on == 'key' else self.name
                        }' """
                        f"is already present in table '{table.name}'."
                    )
E                   sqlalchemy.exc.DuplicateColumnError: A column with name 'group_id' is already present in table 'product_mcve'.

../venv/lib/python3.11/site-packages/sqlalchemy/sql/schema.py:2181: DuplicateColumnError

Stacktrace when running cli alembic upgrade head

Traceback (most recent call last):
  File "/usr/local/bin/alembic", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.11/site-packages/alembic/config.py", line 591, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/usr/local/lib/python3.11/site-packages/alembic/config.py", line 585, in main
    self.run_cmd(cfg, options)
  File "/usr/local/lib/python3.11/site-packages/alembic/config.py", line 562, in run_cmd
    fn(
  File "/usr/local/lib/python3.11/site-packages/alembic/command.py", line 378, in upgrade
    script.run_env()
  File "/usr/local/lib/python3.11/site-packages/alembic/script/base.py", line 576, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/usr/local/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 94, in load_python_file
    module = load_module_py(module_id, path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 110, in load_module_py
    spec.loader.exec_module(module)  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/backend/fondion_alembic/env.py", line 80, in <module>
    run_migrations_online()
  File "/backend/fondion_alembic/env.py", line 74, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/usr/local/lib/python3.11/site-packages/alembic/runtime/environment.py", line 868, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/usr/local/lib/python3.11/site-packages/alembic/runtime/migration.py", line 622, in run_migrations
    step.migration_fn(**kw)
  File "/backend/fondion_alembic/versions/57b2cbd83d3a_mcve.py", line 27, in upgrade
    op.create_foreign_key(
  File "<string>", line 8, in create_foreign_key
  File "<string>", line 3, in create_foreign_key
  File "/usr/local/lib/python3.11/site-packages/alembic/operations/ops.py", line 665, in create_foreign_key
    return operations.invoke(op)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/alembic/operations/base.py", line 401, in invoke
    return fn(self, operation)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/alembic/operations/toimpl.py", line 176, in create_constraint
    operation.to_constraint(operations.migration_context)
  File "/usr/local/lib/python3.11/site-packages/alembic/operations/ops.py", line 578, in to_constraint
    return schema_obj.foreign_key_constraint(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/alembic/operations/schemaobj.py", line 89, in foreign_key_constraint
    t1 = sa_schema.Table(
         ^^^^^^^^^^^^^^^^
  File "<string>", line 2, in __new__
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/util/deprecations.py", line 277, in warned
    return fn(*args, **kwargs)  # type: ignore[no-any-return]
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 428, in __new__
    return cls._new(*args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 482, in _new
    with util.safe_reraise():
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/util/langhelpers.py", line 147, in __exit__
    raise exc_value.with_traceback(exc_tb)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 478, in _new
    table.__init__(name, metadata, *args, _no_init=False, **kw)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 866, in __init__
    self._init_items(
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 225, in _init_items
    spwd(self, **kw)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/base.py", line 1300, in _set_parent_with_dispatch
    self._set_parent(parent, **kw)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/schema.py", line 2181, in _set_parent
    raise exc.DuplicateColumnError(
sqlalchemy.exc.DuplicateColumnError: A column with name 'group_id' is already present in table 'product_mcve'.

Versions.

  • OS: Mac OS 13.2.1
  • Python: 3.11
  • Alembic: 1.10.3
  • SQLAlchemy: 2.0.7
  • Database: Postgres 12.11
  • DBAPI: psycopg2==2.9.5

Additional context

One fix is to remove duplicates from the list of columns in here https://github.com/sqlalchemy/alembic/blob/main/alembic/operations/schemaobj.py#L79 t1_cols = set(local_cols + remote_cols), but I'm lacking Alembic/SA knowledge to know if this breaks something else.

Have a nice day!

@Kalmis Kalmis added the requires triage New issue that requires categorization label Apr 6, 2023
@CaselIT
Copy link
Member

CaselIT commented Apr 7, 2023

Hi

It would be helpful if the complete stack strace was posted. I'll try reproducing.

@CaselIT CaselIT added bug Something isn't working op directives and removed requires triage New issue that requires categorization labels Apr 7, 2023
@Kalmis
Copy link
Author

Kalmis commented Apr 7, 2023

Thanks for the hard work! Sure, I added another stack trace which is produced by the alembic upgrade head cli command. I left also the original stack trace as well which is produced by running the migrations via our custom test setup in VS Code, hence the different format.

@zzzeek
Copy link
Member

zzzeek commented Apr 7, 2023

this is likely some "we have to check for the column before adding it" thing in schemaobj.py

@zzzeek
Copy link
Member

zzzeek commented Apr 7, 2023

it's just we have tests for this already and they do not produce a warning or error. so.

@zzzeek
Copy link
Member

zzzeek commented Apr 7, 2023

oh. same name col in both sides. duh

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

uniquify cols for FK table object https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4552

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working op directives
Projects
None yet
Development

No branches or pull requests

4 participants