From: Mike Bayer Date: Wed, 31 Mar 2021 17:16:04 +0000 (-0400) Subject: Expand sibling tests for overlaps warning X-Git-Tag: rel_1_4_5~13^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b99f8cad8d290bfa123742eafa9d381cb7644cd1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Expand sibling tests for overlaps warning Scaled back the warning message added in :ticket:`5171` to not warn for overlapping columns in an inheritance scenario where a particular relationship is local to a subclass and therefore does not represent an overlap. Add errors documentation for the warning and also expand ``util.warn()`` to include a code parameter. Fixes: #6171 Change-Id: Icb1f12d8d645d439ffd2bbb7371c6b00042b6ae3 --- diff --git a/doc/build/changelog/unreleased_14/6171.rst b/doc/build/changelog/unreleased_14/6171.rst new file mode 100644 index 0000000000..608a77ba02 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6171.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 6171 + + Scaled back the warning message added in :ticket:`5171` to not warn for + overlapping columns in an inheritance scenario where a particular + relationship is local to a subclass and therefore does not represent an + overlap. diff --git a/doc/build/errors.rst b/doc/build/errors.rst index 814f62a67c..3bd7b7a5b8 100644 --- a/doc/build/errors.rst +++ b/doc/build/errors.rst @@ -1116,6 +1116,83 @@ message for details. :ref:`error_bbf0` +.. _error_qzyx: + +relationship X will copy column Q to column P, which conflicts with relationship(s): 'Y' +---------------------------------------------------------------------------------------- + +This warning refers to the case when two or more relationships will write data to the +same columns on flush, but the ORM does not have any kind of back population configuration +between the two relationships. The fix is usually to install the correct +:paramref:`_orm.back_populates` configuration. Given the following mapping:: + + class Parent(Base): + __tablename__ = "parent" + id = Column(Integer, primary_key=True) + children = relationship("Child") + + + class Child(Base): + __tablename__ = "child" + id = Column(Integer, primary_key=True) + parent_id = Column(ForeignKey("parent.id")) + parent = relationship("Parent") + +The above mapping will generate warnings:: + + SAWarning: relationship 'Child.parent' will copy column parent.id to column child.parent_id, + which conflicts with relationship(s): 'Parent.children' (copies parent.id to child.parent_id). + +The relationships ``Child.parent`` and ``Parent.children`` appear to be in conflict. +The solution is to apply :paramref:`_orm.relationship.back_populates`:: + + class Parent(Base): + __tablename__ = "parent" + id = Column(Integer, primary_key=True) + children = relationship("Child", back_populates="parent") + + + class Child(Base): + __tablename__ = "child" + id = Column(Integer, primary_key=True) + parent_id = Column(ForeignKey("parent.id")) + parent = relationship("Parent", back_populates="children") + +For more customized relationships where an "overlap" situation may be +intentional and cannot be resolved, the :paramref:`_orm.relationship.overlaps` +parameter may specify the names of relationships for which the warning should +not take effect. This typically occurs for two or more relationships to the +same underlying table that include custom +:paramref:`_orm.relationship.primaryjoin` conditions that limit the related +items in each case:: + + class Parent(Base): + __tablename__ = "parent" + id = Column(Integer, primary_key=True) + c1 = relationship( + "Child", + primaryjoin="and_(Parent.id == Child.parent_id, Child.flag == 0)", + backref="parent", + overlaps="c2, parent" + ) + c2 = relationship( + "Child", + primaryjoin="and_(Parent.id == Child.parent_id, Child.flag == 1)", + overlaps="c1, parent" + ) + + + class Child(Base): + __tablename__ = "child" + id = Column(Integer, primary_key=True) + parent_id = Column(ForeignKey("parent.id")) + + flag = Column(Integer) + + +Above, the ORM will know that the overlap between ``Parent.c1``, +``Parent.c2`` and ``Child.parent`` is intentional. + AsyncIO Exceptions ================== diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index c1fcc63c1e..2ed9d859ab 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -364,6 +364,10 @@ class RelationshipProperty(StrategizedProperty): .. versionadded:: 1.4 + .. seealso:: + + :ref:`error_qzyx` - usage example + :param bake_queries=True: Use the :class:`.BakedQuery` cache to cache the construction of SQL used in lazy loads. True by default. Set to False if the @@ -3423,6 +3427,8 @@ class JoinCondition(object): and self.prop.key not in pr._overlaps and not self.prop.parent.is_sibling(pr.parent) and not self.prop.mapper.is_sibling(pr.mapper) + and not self.prop.parent.is_sibling(pr.mapper) + and not self.prop.mapper.is_sibling(pr.parent) and ( self.prop.key != pr.key or not self.prop.parent.common_parent(pr.parent) @@ -3453,7 +3459,8 @@ class JoinCondition(object): "'%s' (copies %s to %s)" % (pr, fr_, to_) for (pr, fr_) in other_props ), - ) + ), + code="qzyx", ) self._track_overlapping_sync_targets[to_][self.prop] = from_ diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index 51b9071d56..b31f316fee 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -1604,13 +1604,16 @@ class _hash_limit_string(compat.text_type): return hash(self) == hash(other) -def warn(msg): +def warn(msg, code=None): """Issue a warning. If msg is a string, :class:`.exc.SAWarning` is used as the category. """ + if code: + msg = "%s %s" % (msg, exc.SQLAlchemyError(msg, code=code)._code_str()) + warnings.warn(msg, exc.SAWarning, stacklevel=2) diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 8d73cd40e1..a065b4046a 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -802,6 +802,29 @@ class OverlappingFksSiblingTest(fixtures.TestBase): configure_mappers() + def _fixture_four(self): + Base = declarative_base(metadata=self.metadata) + + class A(Base): + __tablename__ = "a" + + id = Column(Integer, primary_key=True) + + c_id = Column(ForeignKey("c.id")) + + class B1(A): + pass + + class B2(A): + pass + + class C(Base): + __tablename__ = "c" + + id = Column(Integer, primary_key=True) + b1 = relationship(B1, backref="c") + b2 = relationship(B2, backref="c") + @testing.provide_metadata def _test_fixture_one_run(self, **kw): A, AMember, B, BSub1, BSub2 = self._fixture_one(**kw) @@ -853,6 +876,10 @@ class OverlappingFksSiblingTest(fixtures.TestBase): setup_backrefs=False, ) + @testing.provide_metadata + def test_fixture_four(self): + self._fixture_four() + @testing.provide_metadata def test_simple_backrefs_works(self): self._fixture_two(setup_backrefs=True)