From: Mike Bayer Date: Mon, 16 Jan 2017 18:34:55 +0000 (-0500) Subject: Annotate parentmapper in primaryjoin / secondaryjoin X-Git-Tag: rel_1_2_0b1~148 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=63a7b2d2d9402b06f9bc7745eed2d98ae9f8b11c;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Annotate parentmapper in primaryjoin / secondaryjoin This patch applies the "parentmapper" annotation to the columns in the primaryjoin/secondaryjoin, but more dramatically, also removes all the "deannotate" steps that were historically applied to the relationship primaryjoin/secondaryjoin. These deannotation steps were left over from the initial implementations of annotations where the behaviors were not as reliable. By ensuring these annotations are present, the evaluator no longer needs to do a name-based lookup when it sees a column that has no "parentmapper", because it can be assured this is not a mapped column. This fixes the issue where the expression were based on a relationship primaryjoin but the name of a column in the join condition didn't match the attribute name. Change-Id: I8c1d4594116d4109fef314a87c96a24d2efa0058 Fixes: #3366 --- diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index f4fd6a3fe8..daaa11cfed 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -13,6 +13,17 @@ .. changelog:: :version: 1.2.0b1 + .. change:: 3366 + :tags: bug, orm + :tickets: 3366 + + The "evaluate" strategy used by :meth:`.Query.update` and + :meth:`.Query.delete` can now accommodate a simple + object comparison from a many-to-one relationship to an instance, + when the attribute names of the primary key / foreign key columns + don't match the actual names of the columns. Previously this would + do a simple name-based match and fail with an AttributeError. + .. change:: 3938 :tags: bug, engine :tickets: 3938 diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py index 95a9e9b915..aff6718d31 100644 --- a/lib/sqlalchemy/orm/evaluator.py +++ b/lib/sqlalchemy/orm/evaluator.py @@ -59,7 +59,9 @@ class EvaluatorCompiler(object): ) key = parentmapper._columntoproperty[clause].key else: - key = clause.key + raise UnevaluatableError( + "Cannot evaluate column: %s" % clause + ) get_corresponding_attr = operator.attrgetter(key) return lambda obj: get_corresponding_attr(obj) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 1298e8880a..dacb68b453 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -1727,8 +1727,8 @@ class RelationshipProperty(StrategizedProperty): support_sync=not self.viewonly, can_be_synced_fn=self._columns_are_mapped ) - self.primaryjoin = jc.deannotated_primaryjoin - self.secondaryjoin = jc.deannotated_secondaryjoin + self.primaryjoin = jc.primaryjoin + self.secondaryjoin = jc.secondaryjoin self.direction = jc.direction self.local_remote_pairs = jc.local_remote_pairs self.remote_side = jc.remote_columns @@ -1987,6 +1987,7 @@ class JoinCondition(object): self._annotate_fks() self._annotate_remote() self._annotate_local() + self._annotate_parentmapper() self._setup_pairs() self._check_foreign_cols(self.primaryjoin, True) if self.secondaryjoin is not None: @@ -2450,6 +2451,19 @@ class JoinCondition(object): self.primaryjoin, {}, locals_ ) + def _annotate_parentmapper(self): + if self.prop is None: + return + + def parentmappers_(elem): + if "remote" in elem._annotations: + return elem._annotate({"parentmapper": self.prop.mapper}) + elif "local" in elem._annotations: + return elem._annotate({"parentmapper": self.prop.parent}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, parentmappers_ + ) + def _check_remote_side(self): if not self.local_remote_pairs: raise sa_exc.ArgumentError( @@ -2707,17 +2721,6 @@ class JoinCondition(object): def foreign_key_columns(self): return self._gather_join_annotations("foreign") - @util.memoized_property - def deannotated_primaryjoin(self): - return _deep_deannotate(self.primaryjoin) - - @util.memoized_property - def deannotated_secondaryjoin(self): - if self.secondaryjoin is not None: - return _deep_deannotate(self.secondaryjoin) - else: - return None - def _gather_join_annotations(self, annotation): s = set( self._gather_columns_with_annotation( @@ -2856,9 +2859,6 @@ class JoinCondition(object): bind_to_col = dict((binds[col].key, col) for col in binds) - # this is probably not necessary - lazywhere = _deep_deannotate(lazywhere) - return lazywhere, bind_to_col, equated_columns diff --git a/test/orm/test_evaluator.py b/test/orm/test_evaluator.py index d31610f2b6..ffb8d8701b 100644 --- a/test/orm/test_evaluator.py +++ b/test/orm/test_evaluator.py @@ -9,6 +9,14 @@ from sqlalchemy import and_, or_, not_ from sqlalchemy.orm import evaluator from sqlalchemy.orm import mapper +from sqlalchemy.orm import relationship, Session +from sqlalchemy import ForeignKey +from sqlalchemy.orm import exc as orm_exc +from sqlalchemy.testing import assert_raises_message +from sqlalchemy.testing import is_ +from sqlalchemy import inspect + + compiler = evaluator.EvaluatorCompiler() @@ -76,6 +84,21 @@ class EvaluateTest(fixtures.MappedTest): eval_eq(User.name == None, # noqa testcases=[(User(name='foo'), False), (User(name=None), True)]) + def test_raise_on_unannotated_column(self): + User = self.classes.User + + assert_raises_message( + evaluator.UnevaluatableError, + "Cannot evaluate column: foo", + compiler.process, User.id == Column('foo', Integer) + ) + + # if we let the above method through as we did + # prior to [ticket:3366], we would get + # AttributeError: 'User' object has no attribute 'foo' + # u1 = User(id=5) + # meth(u1) + def test_true_false(self): User = self.classes.User @@ -134,3 +157,39 @@ class EvaluateTest(fixtures.MappedTest): (User(id=None, name='foo'), None), (User(id=None, name=None), None), ]) + + +class M2OEvaluateTest(fixtures.DeclarativeMappedTest): + @classmethod + def setup_classes(cls): + Base = cls.DeclarativeBasic + + class Parent(Base): + __tablename__ = "parent" + id = Column(Integer, primary_key=True) + + class Child(Base): + __tablename__ = "child" + _id_parent = Column( + "id_parent", Integer, ForeignKey(Parent.id), primary_key=True) + name = Column(String(50), primary_key=True) + parent = relationship(Parent) + + def test_delete(self): + Parent, Child = self.classes('Parent', 'Child') + + session = Session() + + p = Parent(id=1) + session.add(p) + session.commit() + + c = Child(name="foo", parent=p) + session.add(c) + session.commit() + + session.query(Child).filter(Child.parent == p).delete("evaluate") + + is_( + inspect(c).deleted, True + )