From ef327adcde54ffe19a40c7bf712b35184a696923 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 14 Jun 2023 10:51:43 -0400 Subject: [PATCH] warn for other mapper property objects assigned twice was already in place for columns via other means A warning is emitted when an ORM :func:`_orm.relationship` and other :class:`.MapperProperty` objects are assigned to two different class attributes at once; only one of the attributes will be mapped. A warning for this condition was already in place for :class:`_schema.Column` and :class:`_orm.mapped_column` objects. Fixes: #3532 Change-Id: Ib8057bdf229aa92137f9b8f61b26d4008181ead6 --- doc/build/changelog/unreleased_20/3532.rst | 10 +++ lib/sqlalchemy/orm/mapper.py | 9 ++ test/orm/declarative/test_basic.py | 96 ++++++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 doc/build/changelog/unreleased_20/3532.rst diff --git a/doc/build/changelog/unreleased_20/3532.rst b/doc/build/changelog/unreleased_20/3532.rst new file mode 100644 index 0000000000..9e11de9434 --- /dev/null +++ b/doc/build/changelog/unreleased_20/3532.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm, declarative + :tickets: 3532 + + A warning is emitted when an ORM :func:`_orm.relationship` and other + :class:`.MapperProperty` objects are assigned to two different class + attributes at once; only one of the attributes will be mapped. A warning + for this condition was already in place for :class:`_schema.Column` and + :class:`_orm.mapped_column` objects. + diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index a54c9e9707..cf9256f63e 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2154,6 +2154,15 @@ class Mapper( for proxy_col in col.proxy_set: self._columntoproperty[proxy_col] = prop + if getattr(prop, "key", key) != key: + util.warn( + f"ORM mapped property {self.class_.__name__}.{prop.key} being " + "assigned to attribute " + f"{key!r} is already associated with " + f"attribute {prop.key!r}. The attribute will be de-associated " + f"from {prop.key!r}." + ) + prop.key = key if setparent: diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py index 5ba1034543..985b600f0d 100644 --- a/test/orm/declarative/test_basic.py +++ b/test/orm/declarative/test_basic.py @@ -10,6 +10,7 @@ from sqlalchemy import ForeignKeyConstraint from sqlalchemy import Index from sqlalchemy import inspect from sqlalchemy import Integer +from sqlalchemy import literal from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing @@ -39,6 +40,7 @@ from sqlalchemy.orm import Mapper from sqlalchemy.orm import registry from sqlalchemy.orm import relationship from sqlalchemy.orm import Session +from sqlalchemy.orm import synonym from sqlalchemy.orm import synonym_for from sqlalchemy.orm.decl_api import add_mapped_attribute from sqlalchemy.orm.decl_api import DeclarativeBaseNoMeta @@ -1441,6 +1443,100 @@ class DeclarativeMultiBaseTest( y = Column("y", Integer) + 5 z = "im not a sqlalchemy thing" + @testing.variation( + "attr_type", + [ + "column", + "mapped_column", + "relationship", + "synonym", + "column_property", + ], + ) + def test_attr_assigned_to_multiple_keys(self, attr_type, decl_base): + """test #3532""" + + column_warning = expect_warnings( + "On class 'A', Column object 'a' named directly multiple " + "times, only one will be used: a, b. Consider using " + "orm.synonym instead" + ) + + other_warning = expect_warnings( + "ORM mapped property A.a being assigned to attribute 'b' is " + "already associated with attribute 'a'. The attribute will be " + "de-associated from 'a'." + ) + if attr_type.column: + with column_warning: + + class A(decl_base): + __tablename__ = "a" + + id = Column(Integer, primary_key=True) + + a = Column(Integer) + + b = a + + elif attr_type.mapped_column: + with column_warning: + + class A(decl_base): + __tablename__ = "a" + + id = mapped_column(Integer, primary_key=True) + + a = mapped_column(Integer) + + b = a + + elif attr_type.relationship: + with other_warning: + + class B(decl_base): + __tablename__ = "b" + + id = mapped_column(Integer, primary_key=True) + aid = mapped_column(ForeignKey("a.id")) + + class A(decl_base): + __tablename__ = "a" + + id = mapped_column(Integer, primary_key=True) + + a = relationship("B") + + b = a + + decl_base.registry.configure() + elif attr_type.column_property: + with other_warning: + + class A(decl_base): + __tablename__ = "a" + + id = mapped_column(Integer, primary_key=True) + + a = column_property(literal("foo") + literal("bar")) + + b = a + + elif attr_type.synonym: + with other_warning: + + class A(decl_base): + __tablename__ = "a" + + id = mapped_column(Integer, primary_key=True) + g = mapped_column(Integer) + a = synonym("g") + + b = a + + else: + attr_type.fail() + def test_column_named_twice(self): with expect_warnings( "On class 'Foo', Column object 'x' named directly multiple " -- 2.39.5