sqlalchemy / sqlalchemy/sqlalchemy

`NoReferencedColumnError` when using `referred_column_0[[_]N]_name` in FK naming convention

Open
#5,350 6 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug code review in progress naming conventions schema
Dominant language
Python
Stars
12.2k
Forks
1.8k
PR merge metrics
No merged PRs in 30d

Description

I've read https://github.com/sqlalchemy/sqlalchemy/issues/3989 but this seems unrelated.

When defining a ForeignKey on a SQLAlchemy model, if the referenced column is given as a string ({table_name}.{column_name}) and the Base.metadata has been setup with a naming convention that uses any of thereferred_column_0[[_]N]_name tokens for the foreign key convention, then an sqlalchemy.exc.NoReferencedColumnError is raised when the model class is being declared/mapped. Declaring the ForeignKey by passing a Column object works, though.

This sample code triggers the bug:

import sqlalchemy as sa
from sqlalchemy import MetaData
from sqlalchemy.ext.declarative import declarative_base


metadata = MetaData(naming_convention={
    "fk": "fk__%(referred_table_name)s__%(referred_column_0_name)s",
})
Base = declarative_base(metadata=metadata)

class Parent(Base):
    __tablename__ = 'parent'
    id = sa.Column(sa.Integer, autoincrement=True, primary_key=True)
    parent_id = sa.Column(sa.Integer, sa.ForeignKey('parent.id'))

If I change the last line to

    parent_id = sa.Column(sa.Integer, sa.ForeignKey(id))

it works as expected. But referencing the column directly is not always possible. Although, in those cases, declaring the ForeignKeyConstraint separately via __table_args__ works like a charm.

Error

Traceback (most recent call last):
  File "/home/thomas/dev/python/sqlalchemy_test/naming.py", line 17, in <module>
    class Child(Base):
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/ext/declarative/api.py", line 75, in __init__
    _as_declarative(cls, classname, cls.__dict__)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/ext/declarative/base.py", line 131, in _as_declarative
    _MapperConfig.setup_mapping(cls, classname, dict_)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/ext/declarative/base.py", line 160, in setup_mapping
    cfg_cls(cls_, classname, dict_)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/ext/declarative/base.py", line 190, in __init__
    self._setup_table()
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/ext/declarative/base.py", line 534, in _setup_table
    cls.__table__ = table = table_cls(
  File "<string>", line 2, in __new__
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/util/deprecations.py", line 128, in warned
    return fn(*args, **kwargs)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 507, in __new__
    metadata._remove_table(name, schema)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py", line 68, in __exit__
    compat.raise_(
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 178, in raise_
    raise exception
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 502, in __new__
    table._init(name, metadata, *args, **kw)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 601, in _init
    self._init_items(*args)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 118, in _init_items
    spwd(self)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/base.py", line 460, in _set_parent_with_dispatch
    self.dispatch.after_parent_attach(self, parent)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 322, in __call__
    fn(*args, **kw)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 2111, in _set_table
    self.constraint._set_parent_with_dispatch(table)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/base.py", line 460, in _set_parent_with_dispatch
    self.dispatch.after_parent_attach(self, parent)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 261, in __call__
    fn(*args, **kw)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/naming.py", line 180, in _constraint_name
    newname = _constraint_name_for_table(const, table)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/naming.py", line 157, in _constraint_name_for_table
    convention
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/naming.py", line 108, in __getitem__
    tokens.append(getattr(self, attr)(idx))
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/naming.py", line 86, in _key_referred_column_X_name
    return fk.column.name
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py", line 883, in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
  File "/home/thomas/dev/python/sqlalchemy_test/.env/lib/python3.8/site-packages/sqlalchemy/sql/schema.py", line 2057, in column
    raise exc.NoReferencedColumnError(
sqlalchemy.exc.NoReferencedColumnError: Could not initialize target column for ForeignKey 'parent.id' on table 'child': table 'parent' has no column named 'id'

After some investigating, I realized the the naming convention was applied before the ForeignKey had been associated to its referenced Column so, as a test, I made this rather crude modification to delay the _set_parent_with_dispatch after the Column had been associated. It works both on 1.3.16 and master, but my knowledge of SQLAlchemy's internals is not good, and the fix seems to delay a huge chunk of logic (a lot more than just applying the naming convention). I'm posting it here in case it can help in finding an actual fix.

diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 689eda11d..4245a7f45 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -2088,6 +2088,7 @@ class ForeignKey(DialectKWArgs, SchemaItem):
         # standalone ForeignKey - create ForeignKeyConstraint
         # on the hosting Table when attached to the Table.
         assert isinstance(table, Table)
+        created = False
         if self.constraint is None:
             self.constraint = ForeignKeyConstraint(
                 [],
@@ -2102,7 +2103,8 @@ class ForeignKey(DialectKWArgs, SchemaItem):
                 **self._unvalidated_dialect_kw
             )
             self.constraint._append_element(column, self)
-            self.constraint._set_parent_with_dispatch(table)
+            created = True
+
         table.foreign_keys.add(self)
         # set up remote ".column" attribute, or a note to pick it
         # up when the other Table/Column shows up
@@ -2124,6 +2126,8 @@ class ForeignKey(DialectKWArgs, SchemaItem):
             _column = self._colspec
             self._set_target_column(_column)
 
+        if created:
+            self.constraint._set_parent_with_dispatch(table)
 
 class DefaultGenerator(SchemaItem):
     """Base class for column *default* values."""

Otherwise, a least-side-effect-possible solution would be to detect the missing column inside of sqlalchemy.sql.naming.ConventionDict._key_referred_column_X_name, and raise a clearer error. Something like this (pseudo-code, just to illustrate):

def _key_referred_column_X_name(self, idx):
    fk = self.const.elements[idx]
    try:
        return fk.column.name
    except NoReferencedColumnError as err:
        raise NamingConventionError(
            'Token column_0_name cannot be used with ForeignKey(str)'
        ) from err

Versions

  • OS: Manjaro Linux
  • Python: 3.8.2
  • SQLAlchemy: 1.3.16 + master (both for the bug and the ugly fix)
  • Database: None
  • DBAPI: None

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the sample with a string ForeignKey and the referred_column_0_name convention, then read lib/sqlalchemy/sql/schema.py around ForeignKey._set_parent_with_dispatch and sqlalchemy.sql.naming.ConventionDict._key_referred_column_X_name. Compare the reported ordering change with the existing ForeignKeyConstraint path. Done means the sample no longer raises NoReferencedColumnError while naming conventions and direct Column references continue to work.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.