From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sun, 2 Aug 2009 18:13:07 +0000 (+0000)
Subject: - UPDATE and DELETE do not support ORDER BY, LIMIT, OFFSET,
X-Git-Tag: rel_0_5_6~16
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5cac19a9c047ac2fbb4dc283047e4d93e44210bf;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- UPDATE and DELETE do not support ORDER BY, LIMIT, OFFSET,
etc. in standard SQL.  Query.update() and Query.delete()
now raise an exception if any of limit(), offset(),
order_by(), group_by(), or distinct() have been
called. [ticket:1487]
---

diff --git a/CHANGES b/CHANGES
index 447577f9de..3ff1ee032b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -37,6 +37,12 @@ CHANGES
       inheritance setups - attribute extensions won't randomly
       collide with each other.  [ticket:1488]
     
+    - UPDATE and DELETE do not support ORDER BY, LIMIT, OFFSET,
+      etc. in standard SQL.  Query.update() and Query.delete()
+      now raise an exception if any of limit(), offset(),
+      order_by(), group_by(), or distinct() have been
+      called. [ticket:1487]
+      
     - Added AttributeExtension to sqlalchemy.orm.__all__
       
     - Improved error message when query() is called with
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 1574545eb7..e764856bf2 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -316,6 +316,21 @@ class Query(object):
                 "Otherwise, call %s() before limit() or offset() are applied." % (meth, meth)
             )
 
+    def _no_select_modifiers(self, meth):
+        if not self._enable_assertions:
+            return
+        for attr, methname, notset in (
+            ('_limit', 'limit()', None),
+            ('_offset', 'offset()', None),
+            ('_order_by', 'order_by()', False),
+            ('_group_by', 'group_by()', False),
+            ('_distinct', 'distinct()', False),
+        ):
+            if getattr(self, attr) is not notset:
+                raise sa_exc.InvalidRequestError(
+                    "Can't call Query.%s() when %s has been called" % (meth, methname)
+                )
+
     def _get_options(self, populate_existing=None, 
                             version_check=None, 
                             only_load_props=None, 
@@ -1613,6 +1628,7 @@ class Query(object):
 
         if synchronize_session not in [False, 'evaluate', 'fetch']:
             raise sa_exc.ArgumentError("Valid strategies for session synchronization are False, 'evaluate' and 'fetch'")
+        self._no_select_modifiers("delete")
 
         self = self.enable_eagerloads(False)
 
@@ -1707,6 +1723,7 @@ class Query(object):
         #TODO: updates of manytoone relations need to be converted to fk assignments
         #TODO: cascades need handling.
 
+        self._no_select_modifiers("update")
         if synchronize_session not in [False, 'evaluate', 'expire']:
             raise sa_exc.ArgumentError("Valid strategies for session synchronization are False, 'evaluate' and 'expire'")
 
diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 67e933efb7..88a95bf760 100644
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -2843,6 +2843,22 @@ class UpdateDeleteTest(_base.MappedTest):
             'user': relation(User, lazy=False, backref=backref('documents', lazy=True))
         })
 
+    @testing.resolve_artifact_names
+    def test_illegal_operations(self):
+        s = create_session()
+        
+        for q, mname in (
+            (s.query(User).limit(2), "limit"),
+            (s.query(User).offset(2), "offset"),
+            (s.query(User).limit(2).offset(2), "limit"),
+            (s.query(User).order_by(User.id), "order_by"),
+            (s.query(User).group_by(User.id), "group_by"),
+            (s.query(User).distinct(), "distinct")
+        ):
+            assert_raises_message(sa_exc.InvalidRequestError, r"Can't call Query.update\(\) when %s\(\) has been called" % mname, q.update, {'name':'ed'})
+            assert_raises_message(sa_exc.InvalidRequestError, r"Can't call Query.delete\(\) when %s\(\) has been called" % mname, q.delete)
+            
+        
     @testing.resolve_artifact_names
     def test_delete(self):
         sess = create_session(bind=testing.db, autocommit=False)