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

diff --git a/doc/build/changelog/unreleased_14/6400.rst b/doc/build/changelog/unreleased_14/6400.rst
new file mode 100644 (file)
index 0000000..928feb9
--- /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 fc2c77f0d01857c9f84a284188f76202ddebea9e..8034475e8309ccbd4ba187fc5f9675d296889502 100644 (file)
--- 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 efdd7edf101616afe41b6588904fdb294f7c4294..171df3e21ac8d019a0e39d0b0bd5da7b5694b32c 100644 (file)
--- 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 77a218bebd76167d08a4797c229b49dd9d1e77ac..e14db9f8031418f65a021b2fd9049f3835635c25 100644 (file)
--- 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()