- 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

5 files changed, 128 insertions, 10 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 3d29c9eb4..9767567c6 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -22,6 +22,20 @@
     :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 c317b734a..2deeda376 100644
--- 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 95aa14a26..88dadcc22 100644
--- 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 f822071c4..9b02d86e9 100644
--- 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 573913f74..2539d4737 100644
--- 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."""