From 0c46a5753329a4f82784ccc457ba1a0e750815b9 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 1 Jun 2021 16:03:52 -0400 Subject: [PATCH] improve "overlaps" warning; test for m2m The warning that's emitted for :func:`_orm.relationship` when multiple relationships would overlap with each other as far as foreign key attributes written towards, now includes the specific "overlaps" argument to use for each warning in order to silence the warning without changing the mapping. Fixes: #6400 Change-Id: I43c04f8018dda4e0f83ae58b8c7fd95084d9e95d --- doc/build/changelog/unreleased_14/6400.rst | 9 +++ doc/build/errors.rst | 16 +++-- lib/sqlalchemy/orm/relationships.py | 7 ++- test/orm/test_relationships.py | 70 +++++++++++++++++++++- 4 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6400.rst diff --git a/doc/build/changelog/unreleased_14/6400.rst b/doc/build/changelog/unreleased_14/6400.rst new file mode 100644 index 0000000000..928feb9bd9 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6400.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 6400 + + The warning that's emitted for :func:`_orm.relationship` when multiple + relationships would overlap with each other as far as foreign key + attributes written towards, now includes the specific "overlaps" argument + to use for each warning in order to silence the warning without changing + the mapping. diff --git a/doc/build/errors.rst b/doc/build/errors.rst index fc2c77f0d0..8034475e83 100644 --- a/doc/build/errors.rst +++ b/doc/build/errors.rst @@ -1153,10 +1153,18 @@ message for details. 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.relationship.back_populates` configuration. Given the following mapping:: +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 means of +coordinating these relationships together. Depending on specifics, the solution +may be that two relationships need to be referred towards one another using +:paramref:`_orm.relationship.back_populates`, or that one or more of the +relationships should be configured with :paramref:`_orm.relationship.viewonly` +to prevent conflicting writes, or sometimes that the configuration is fully +intentional and should configure :paramref:`_orm.relationship.overlaps` to +silence each warning. + +For the typical example that's missing +:paramref:`_orm.relationship.back_populates`, given the following mapping:: class Parent(Base): __tablename__ = "parent" diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index efdd7edf10..171df3e21a 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -3451,8 +3451,9 @@ class JoinCondition(object): "constraints are partially overlapping, the " "orm.foreign() " "annotation can be used to isolate the columns that " - "should be written towards. The 'overlaps' " - "parameter may be used to remove this warning." + "should be written towards. To silence this " + "warning, add the parameter 'overlaps=\"%s\"' to the " + "'%s' relationship." % ( self.prop, from_, @@ -3461,6 +3462,8 @@ class JoinCondition(object): "'%s' (copies %s to %s)" % (pr, fr_, to_) for (pr, fr_) in other_props ), + ",".join(pr.key for pr, fr in other_props), + self.prop, ), code="qzyx", ) diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index 77a218bebd..e14db9f803 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -12,7 +12,6 @@ from sqlalchemy import MetaData from sqlalchemy import select from sqlalchemy import String from sqlalchemy import testing -from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import aliased from sqlalchemy.orm import attributes from sqlalchemy.orm import backref @@ -20,6 +19,7 @@ from sqlalchemy.orm import clear_mappers from sqlalchemy.orm import column_property from sqlalchemy.orm import composite from sqlalchemy.orm import configure_mappers +from sqlalchemy.orm import declarative_base from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import foreign from sqlalchemy.orm import joinedload @@ -39,6 +39,7 @@ from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing import in_ from sqlalchemy.testing import is_ +from sqlalchemy.testing.assertions import expect_warnings from sqlalchemy.testing.assertsql import assert_engine from sqlalchemy.testing.assertsql import CompiledSQL from sqlalchemy.testing.fixtures import fixture_session @@ -876,6 +877,73 @@ class OverlappingFksSiblingTest(fixtures.TestBase): setup_backrefs=False, ) + @testing.combinations((True,), (False,), argnames="set_overlaps") + def test_fixture_five(self, metadata, set_overlaps): + Base = declarative_base(metadata=self.metadata) + + if set_overlaps: + overlaps = "as,cs" + else: + overlaps = None + + class A(Base): + __tablename__ = "a" + + id = Column(Integer, primary_key=True) + cs = relationship("C", secondary="b", backref="as") + bs = relationship("B", back_populates="a", overlaps=overlaps) + + class B(Base): + __tablename__ = "b" + + a_id = Column(ForeignKey("a.id"), primary_key=True) + c_id = Column(ForeignKey("c.id"), primary_key=True) + a = relationship("A", back_populates="bs", overlaps=overlaps) + c = relationship("C", back_populates="bs", overlaps=overlaps) + + class C(Base): + __tablename__ = "c" + + id = Column(Integer, primary_key=True) + bs = relationship("B", back_populates="c", overlaps=overlaps) + + if set_overlaps: + configure_mappers() + else: + with expect_warnings( + r"relationship 'A.bs' will copy column a.id to column b.a_id, " + r"which conflicts with relationship\(s\): " + r"'C.as' \(copies a.id to b.a_id\), " + r"'A.cs' \(copies a.id to b.a_id\)" + r".*add the parameter 'overlaps=\"as,cs\"' to the 'A.bs' " + r"relationship", + # + # + r"relationship 'B.a' will copy column a.id to column b.a_id, " + r"which conflicts with relationship\(s\): " + r"'C.as' \(copies a.id to b.a_id\), " + r"'A.cs' \(copies a.id to b.a_id\)..*" + r"add the parameter 'overlaps=\"as,cs\"' to the 'B.a' " + r"relationship", + # + # + r"relationship 'B.c' will copy column c.id to column b.c_id, " + r"which conflicts with relationship\(s\): " + r"'C.as' \(copies c.id to b.c_id\), " + r"'A.cs' \(copies c.id to b.c_id\)" + r".*add the parameter 'overlaps=\"as,cs\"' to the 'B.c' " + r"relationship", + # + # + r"relationship 'C.bs' will copy column c.id to column b.c_id, " + r"which conflicts with relationship\(s\): " + r"'C.as' \(copies c.id to b.c_id\), " + r"'A.cs' \(copies c.id to b.c_id\)" + r".*add the parameter 'overlaps=\"as,cs\"' to the 'C.bs' " + r"relationship", + ): + configure_mappers() + @testing.provide_metadata def test_fixture_four(self): self._fixture_four() -- 2.47.2