From 0276ec0cd874aafb06606c443e8cdeb692c232f9 Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 17 Jun 2010 15:53:07 -0400
Subject: [PATCH] - query.order_by() now accepts False, which cancels any
 existing order_by() state on the Query, allowing subsequent generative
 methods to be called which do not support ORDER BY.  This is not the same as
 the already existing feature of passing None, which suppresses any existing
 order_by() settings, including those configured on the mapper.  False will
 make it as though order_by() was never called, while None is an active
 setting.

---
 CHANGES                     | 10 ++++++++++
 lib/sqlalchemy/orm/query.py | 35 +++++++++++++++++++++++---------
 test/orm/test_query.py      | 40 +++++++++++++++++++++++--------------
 3 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/CHANGES b/CHANGES
index 84cac88906..b2dd60b54c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -29,6 +29,16 @@ CHANGES
     It now ignores objects that are no longer 
     in the "persistent" state, and the parent's 
     foreign key identifier is left unaffected.
+
+  - query.order_by() now accepts False, which cancels
+    any existing order_by() state on the Query, allowing
+    subsequent generative methods to be called which do 
+    not support ORDER BY.  This is not the same as the 
+    already existing feature of passing None, which 
+    suppresses any existing order_by() settings, including 
+    those configured on the mapper.  False will make it
+    as though order_by() was never called, while
+    None is an active setting.
     
 - sql
   - The warning emitted by the Unicode and String types
diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 50be13088e..6b36b370a6 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -854,17 +854,34 @@ class Query(object):
     @util.accepts_a_list_as_starargs(list_deprecation='deprecated')
     def order_by(self, *criterion):
         """apply one or more ORDER BY criterion to the query and return 
-        the newly resulting ``Query``"""
+        the newly resulting ``Query``
+        
+        All existing ORDER BY settings can be suppressed by 
+        passing ``None`` - this will suppress any ORDER BY configured
+        on mappers as well.
+        
+        Alternatively, an existing ORDER BY setting on the Query
+        object can be entirely cancelled by passing ``False`` 
+        as the value - use this before calling methods where
+        an ORDER BY is invalid.
+        
+        """
 
-        if len(criterion) == 1 and criterion[0] is None:
-            self._order_by = None
-        else:
-            criterion = self._adapt_col_list(criterion)
+        if len(criterion) == 1:
+            if criterion[0] is False:
+                if '_order_by' in self.__dict__:
+                    del self._order_by
+                return
+            if criterion[0] is None:
+                self._order_by = None
+                return
+                
+        criterion = self._adapt_col_list(criterion)
 
-            if self._order_by is False or self._order_by is None:
-                self._order_by = criterion
-            else:
-                self._order_by = self._order_by + criterion
+        if self._order_by is False or self._order_by is None:
+            self._order_by = criterion
+        else:
+            self._order_by = self._order_by + criterion
 
     @_generative(_no_statement_condition, _no_limit_offset)
     @util.accepts_a_list_as_starargs(list_deprecation='deprecated')
diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 63e9a8de0f..4372ee8408 100644
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -222,21 +222,7 @@ class GetTest(QueryTest):
             'SELECT users.id AS users_id, users.name AS users_name FROM users WHERE users.id = ?'
             )
 
-class OrderByTest(QueryTest, AssertsCompiledSQL):
-    def test_cancel_order_by(self):
-        s = create_session()
-        
-        q = s.query(User).order_by(User.id)
-        self.assert_compile(q, 
-            "SELECT users.id AS users_id, users.name AS users_name FROM users ORDER BY users.id",
-            use_default_dialect=True)
-
-        q = q.order_by(None)
-        self.assert_compile(q, 
-                "SELECT users.id AS users_id, users.name AS users_name FROM users",
-                use_default_dialect=True)
-        
-class InvalidGenerationsTest(QueryTest):
+class InvalidGenerationsTest(QueryTest, AssertsCompiledSQL):
     def test_no_limit_offset(self):
         s = create_session()
         
@@ -316,6 +302,30 @@ class InvalidGenerationsTest(QueryTest):
         assert_raises(sa_exc.InvalidRequestError, q.from_statement, text("select * from table"))
         assert_raises(sa_exc.InvalidRequestError, q.with_polymorphic, User)
         
+    def test_cancel_order_by(self):
+        s = create_session()
+
+        q = s.query(User).order_by(User.id)
+        self.assert_compile(q, 
+            "SELECT users.id AS users_id, users.name AS users_name FROM users ORDER BY users.id",
+            use_default_dialect=True)
+
+        assert_raises(sa_exc.InvalidRequestError, q._no_select_modifiers, "foo")
+
+        q = q.order_by(None)
+        self.assert_compile(q, 
+                "SELECT users.id AS users_id, users.name AS users_name FROM users",
+                use_default_dialect=True)
+
+        assert_raises(sa_exc.InvalidRequestError, q._no_select_modifiers, "foo")
+
+        q = q.order_by(False)
+        self.assert_compile(q, 
+                "SELECT users.id AS users_id, users.name AS users_name FROM users",
+                use_default_dialect=True)
+
+        # after False was set, this should pass
+        q._no_select_modifiers("foo")
         
     def test_mapper_zero(self):
         s = create_session()
-- 
2.47.2