- Fixed issue where two same-named relationships that refer to

a base class and a concrete-inherited subclass would raise an error
if those relationships were set up using "backref", while setting up the
identical configuration using relationship() instead with the conflicting
names would succeed, as is allowed in the case of a concrete mapping.
fixes #3630

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 3d29c9eb43264814d5471d069188e5e601bd01fe..9767567c62454f5b07a498f97647982db315826f 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -21,6 +21,20 @@
 .. changelog::
     :version: 1.1.0b1
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3630
+
+        Fixed issue where two same-named relationships that refer to
+        a base class and a concrete-inherited subclass would raise an error
+        if those relationships were set up using "backref", while setting up the
+        identical configuration using relationship() instead with the conflicting
+        names would succeed, as is allowed in the case of a concrete mapping.
+
+        .. seealso::
+
+            :ref:`change_3630`
+
     .. change::
         :tags: feature, orm
         :tickets: 3081

diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst
index c317b734ae8d0601765edd7edcbe3deb8598d2b1..2deeda37696e200420a976c0867e018c768cdde4 100644 (file)
--- a/doc/build/changelog/migration_11.rst
+++ b/doc/build/changelog/migration_11.rst
@@ -291,6 +291,65 @@ time on the outside of the subquery.
 :ticket:`3582`
 
 
+.. _change_3630:
+
+Same-named backrefs will not raise an error when applied to concrete inheritance subclasses
+-------------------------------------------------------------------------------------------
+
+The following mapping has always been possible without issue::
+
+    class A(Base):
+        __tablename__ = 'a'
+        id = Column(Integer, primary_key=True)
+        b = relationship("B", foreign_keys="B.a_id", backref="a")
+
+    class A1(A):
+        __tablename__ = 'a1'
+        id = Column(Integer, primary_key=True)
+        b = relationship("B", foreign_keys="B.a1_id", backref="a1")
+        __mapper_args__ = {'concrete': True}
+
+    class B(Base):
+        __tablename__ = 'b'
+        id = Column(Integer, primary_key=True)
+
+        a_id = Column(ForeignKey('a.id'))
+        a1_id = Column(ForeignKey('a1.id'))
+
+Above, even though class ``A`` and class ``A1`` have a relationship
+named ``b``, no conflict warning or error occurs because class ``A1`` is
+marked as "concrete".
+
+However, if the relationships were configured the other way, an error
+would occur::
+
+    class A(Base):
+        __tablename__ = 'a'
+        id = Column(Integer, primary_key=True)
+
+
+    class A1(A):
+        __tablename__ = 'a1'
+        id = Column(Integer, primary_key=True)
+        __mapper_args__ = {'concrete': True}
+
+
+    class B(Base):
+        __tablename__ = 'b'
+        id = Column(Integer, primary_key=True)
+
+        a_id = Column(ForeignKey('a.id'))
+        a1_id = Column(ForeignKey('a1.id'))
+
+        a = relationship("A", backref="b")
+        a1 = relationship("A1", backref="b")
+
+The fix enhances the backref feature so that an error is not emitted,
+as well as an additional check within the mapper logic to bypass warning
+for an attribute being replaced.
+
+:ticket:`3630`
+
 .. _change_3601:
 
 Session.merge resolves pending conflicts the same as persistent

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 95aa14a26f803ff530cf509f78c70ea1cca5cc78..88dadcc220072971de0447cbba2bbd2fd8265bf8 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1591,7 +1591,12 @@ class Mapper(InspectionAttr):
 
         if key in self._props and \
                 not isinstance(prop, properties.ColumnProperty) and \
-                not isinstance(self._props[key], properties.ColumnProperty):
+                not isinstance(
+                    self._props[key],
+                    (
+                        properties.ColumnProperty,
+                        properties.ConcreteInheritedProperty)
+                ):
             util.warn("Property %s on %s being replaced with new "
                       "property %s; the old property will be discarded" % (
                           self._props[key],

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index f822071c47965e4dd5bf32d6a67acd860f21ed66..9b02d86e94cf6f5995107c6322cee0f0d055351e 100644 (file)
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -1817,15 +1817,16 @@ class RelationshipProperty(StrategizedProperty):
                 backref_key, kwargs = self.backref
             mapper = self.mapper.primary_mapper()
 
-            check = set(mapper.iterate_to_root()).\
-                union(mapper.self_and_descendants)
-            for m in check:
-                if m.has_property(backref_key):
-                    raise sa_exc.ArgumentError(
-                        "Error creating backref "
-                        "'%s' on relationship '%s': property of that "
-                        "name exists on mapper '%s'" %
-                        (backref_key, self, m))
+            if not mapper.concrete:
+                check = set(mapper.iterate_to_root()).\
+                    union(mapper.self_and_descendants)
+                for m in check:
+                    if m.has_property(backref_key):
+                        raise sa_exc.ArgumentError(
+                            "Error creating backref "
+                            "'%s' on relationship '%s': property of that "
+                            "name exists on mapper '%s'" %
+                            (backref_key, self, m))
 
             # determine primaryjoin/secondaryjoin for the
             # backref.  Use the one we had, so that

diff --git a/test/orm/inheritance/test_concrete.py b/test/orm/inheritance/test_concrete.py
index 573913f74026f41aa84702449a9a45046159500f..2539d4737b8cd444beaaa1f15974290ba486d050 100644 (file)
--- a/test/orm/inheritance/test_concrete.py
+++ b/test/orm/inheritance/test_concrete.py
@@ -486,6 +486,45 @@ class PropertyInheritanceTest(fixtures.MappedTest):
         assert dest1.many_b == [b1, b2]
         assert sess.query(B).filter(B.bname == 'b1').one() is b1
 
+    def test_overlapping_backref_relationship(self):
+        A, B, b_table, a_table, Dest, dest_table = (
+            self.classes.A,
+            self.classes.B,
+            self.tables.b_table,
+            self.tables.a_table,
+            self.classes.Dest,
+            self.tables.dest_table)
+
+        # test issue #3630, no error or warning is generated
+        mapper(A, a_table)
+        mapper(B, b_table, inherits=A, concrete=True)
+        mapper(Dest, dest_table, properties={
+            'a': relationship(A, backref='dest'),
+            'a1': relationship(B, backref='dest')
+        })
+        configure_mappers()
+
+    def test_overlapping_forwards_relationship(self):
+        A, B, b_table, a_table, Dest, dest_table = (
+            self.classes.A,
+            self.classes.B,
+            self.tables.b_table,
+            self.tables.a_table,
+            self.classes.Dest,
+            self.tables.dest_table)
+
+        # this is the opposite mapping as that of #3630, never generated
+        # an error / warning
+        mapper(A, a_table, properties={
+            'dest': relationship(Dest, backref='a')
+        })
+        mapper(B, b_table, inherits=A, concrete=True, properties={
+            'dest': relationship(Dest, backref='a1')
+        })
+        mapper(Dest, dest_table)
+        configure_mappers()
+
+
     def test_polymorphic_backref(self):
         """test multiple backrefs to the same polymorphically-loading
         attribute."""