From: Mike Bayer Date: Tue, 19 Jan 2016 18:34:42 +0000 (-0500) Subject: - Fixed issue where two same-named relationships that refer to X-Git-Tag: rel_1_1_0b1~84^2~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b7bc704f3d05bed8d0771cbff65adcdb7b49f796;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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 3d29c9eb43..9767567c62 100644 --- 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 c317b734ae..2deeda3769 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 95aa14a26f..88dadcc220 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 f822071c47..9b02d86e94 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 573913f740..2539d4737b 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."""