diff options
| -rw-r--r-- | doc/build/changelog/changelog_11.rst | 14 | ||||
| -rw-r--r-- | doc/build/changelog/migration_11.rst | 59 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 7 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/relationships.py | 19 | ||||
| -rw-r--r-- | test/orm/inheritance/test_concrete.py | 39 |
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.""" |