From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 8 Jul 2014 23:08:42 +0000 (-0400)
Subject: - The "evaulator" for query.update()/delete() won't work with multi-table
X-Git-Tag: rel_1_0_0b1~344
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e38bb315fdd2cb327e05630aea805e1c4d8b1325;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- The "evaulator" for query.update()/delete() won't work with multi-table
updates, and needs to be set to `synchronize_session=False` or
`synchronize_session='fetch'`; this now raises an exception, with a
message to change the synchronize setting.  This will be only a warning
in 0.9.7.  fixes #3117
---

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 862d4103ab..de351326ed 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -16,6 +16,16 @@
 .. changelog::
 	:version: 1.0.0
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3117
+
+        The "evaulator" for query.update()/delete() won't work with multi-table
+        updates, and needs to be set to `synchronize_session=False` or
+        `synchronize_session='fetch'`; this now raises an exception, with a
+        message to change the synchronize setting.
+        This is upgraded from a warning emitted as of 0.9.7.
+
     .. change::
         :tags: removed
 
diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py
index e1dd960688..ca26c9ca42 100644
--- a/lib/sqlalchemy/orm/evaluator.py
+++ b/lib/sqlalchemy/orm/evaluator.py
@@ -25,6 +25,9 @@ _notimplemented_ops = set(getattr(operators, op)
 
 
 class EvaluatorCompiler(object):
+    def __init__(self, target_cls=None):
+        self.target_cls = target_cls
+
     def process(self, clause):
         meth = getattr(self, "visit_%s" % clause.__visit_name__, None)
         if not meth:
@@ -46,10 +49,17 @@ class EvaluatorCompiler(object):
 
     def visit_column(self, clause):
         if 'parentmapper' in clause._annotations:
-            key = clause._annotations['parentmapper'].\
-              _columntoproperty[clause].key
+            parentmapper = clause._annotations['parentmapper']
+            if self.target_cls and not issubclass(
+                    self.target_cls, parentmapper.class_):
+                raise UnevaluatableError(
+                    "Can't evaluate criteria against alternate class %s" %
+                        parentmapper.class_
+                )
+            key = parentmapper._columntoproperty[clause].key
         else:
             key = clause.key
+
         get_corresponding_attr = operator.attrgetter(key)
         return lambda obj: get_corresponding_attr(obj)
 
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 996cc88024..56778cb054 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -922,8 +922,10 @@ class BulkEvaluate(BulkUD):
 
     def _do_pre_synchronize(self):
         query = self.query
+        target_cls = query._mapper_zero().class_
+
         try:
-            evaluator_compiler = evaluator.EvaluatorCompiler()
+            evaluator_compiler = evaluator.EvaluatorCompiler(target_cls)
             if query.whereclause is not None:
                 eval_condition = evaluator_compiler.process(
                                                 query.whereclause)
@@ -938,7 +940,6 @@ class BulkEvaluate(BulkUD):
                     "Could not evaluate current criteria in Python. "
                     "Specify 'fetch' or False for the "
                     "synchronize_session parameter.")
-        target_cls = query._mapper_zero().class_
 
         #TODO: detect when the where clause is a trivial primary key match
         self.matched_objects = [
diff --git a/test/orm/test_update_delete.py b/test/orm/test_update_delete.py
index bf72b49f6f..97c7e86b1d 100644
--- a/test/orm/test_update_delete.py
+++ b/test/orm/test_update_delete.py
@@ -21,12 +21,10 @@ class UpdateDeleteTest(fixtures.MappedTest):
                         test_needs_autoincrement=True),
               Column('name', String(32)),
               Column('age', Integer))
-
     @classmethod
     def setup_classes(cls):
         class User(cls.Comparable):
             pass
-
     @classmethod
     def insert_data(cls):
         users = cls.tables.users
@@ -619,6 +617,21 @@ class UpdateDeleteFromTest(fixtures.MappedTest):
                 ])
         )
 
+    def test_no_eval_against_multi_table_criteria(self):
+        User = self.classes.User
+        Document = self.classes.Document
+
+        s = Session()
+
+        q = s.query(User).filter(User.id == Document.user_id)
+        assert_raises_message(
+            exc.InvalidRequestError,
+            "Could not evaluate current criteria in Python.",
+            q.update,
+            {"name": "ed"}
+        )
+
+
     @testing.requires.update_where_target_in_subquery
     def test_update_using_in(self):
         Document = self.classes.Document
@@ -675,7 +688,7 @@ class UpdateDeleteFromTest(fixtures.MappedTest):
             filter(User.id == 2).update({
                     Document.samename: 'd_samename',
                     User.samename: 'u_samename'
-                }
+                }, synchronize_session=False
             )
         eq_(
             s.query(User.id, Document.samename, User.samename).