- query.order_by() accepts None which will remove any pending

order_by state from the query, as well as cancel out any
mapper/relation configured ordering. This is primarily useful
for overriding the ordering specified on a dynamic_loader().
[ticket:1079]

diff --git a/CHANGES b/CHANGES
index e8d6664209cbc6b16b43c2754cf6d27c280bb140..bd0bd870a4d608f7044d1961c87053a01c8e7bde 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -40,7 +40,13 @@ CHANGES
     - dynamic_loader accepts a query_class= to customize the Query
       classes used for both the dynamic collection and the queries
       built from it.
-
+    
+    - query.order_by() accepts None which will remove any pending
+      order_by state from the query, as well as cancel out any
+      mapper/relation configured ordering. This is primarily useful 
+      for overriding the ordering specified on a dynamic_loader().
+      [ticket:1079]
+      
 - sql
     - RowProxy objects can be used in place of dictionary arguments 
       sent to connection.execute() and friends.  [ticket:935]

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index a4e9275e71d261edb12617389029d3ae94d30cdc..e733cc0242956f7976dd7ebb3588cdaeffc73281 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -653,12 +653,15 @@ class Query(object):
     def order_by(self, *criterion):
         """apply one or more ORDER BY criterion to the query and return the newly resulting ``Query``"""
 
-        criterion = [self._adapt_clause(expression._literal_as_text(o), True, True) for o in criterion]
-
-        if self._order_by is False:
-            self._order_by = criterion
+        if len(criterion) == 1 and criterion[0] is None:
+            self._order_by = None
         else:
-            self._order_by = self._order_by + criterion
+            criterion = [self._adapt_clause(expression._literal_as_text(o), True, True) for o in 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='pending')

diff --git a/test/orm/dynamic.py b/test/orm/dynamic.py
index 5c15541401e25e03fa1e5f8cb6764a370f042075..010999ac547c1be277a4a52147f228dfec22bad0 100644 (file)
--- a/test/orm/dynamic.py
+++ b/test/orm/dynamic.py
@@ -42,6 +42,18 @@ class DynamicTest(_fixtures.FixtureTest):
         u = sess.query(User).get(8)
         eq_(list(u.addresses), [Address(email_address=u'ed@wood.com'), Address(email_address=u'ed@lala.com'), Address(email_address=u'ed@bettyboop.com')])
 
+        # test cancellation of None, replacement with something else
+        eq_(
+            list(u.addresses.order_by(None).order_by(Address.email_address)),
+            [Address(email_address=u'ed@bettyboop.com'), Address(email_address=u'ed@lala.com'), Address(email_address=u'ed@wood.com')]
+        )
+
+        # test cancellation of None, replacement with nothing
+        eq_(
+            set(u.addresses.order_by(None)),
+            set([Address(email_address=u'ed@bettyboop.com'), Address(email_address=u'ed@lala.com'), Address(email_address=u'ed@wood.com')])
+        )
+
     @testing.resolve_artifact_names
     def test_count(self):
         mapper(User, users, properties={

diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index d486c3b041fe861e7f771c60b348d43b07ed7be2..2e6b0fed463c07b63e7395771a5dcc201dddadfb 100644 (file)
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -519,20 +519,23 @@ class MapperTest(_fixtures.FixtureTest):
         eq_(l, [self.static.user_result[0]])
 
     @testing.resolve_artifact_names
-    def test_order_by(self):
-        """Ordering at the mapper and query level"""
+    def test_cancel_order_by(self):
+        mapper(User, users, order_by=users.c.name.desc())
 
-        # TODO: make a unit test out of these various combinations
-        #m = mapper(User, users, order_by=desc(users.c.name))
-        mapper(User, users, order_by=None)
-        #mapper(User, users)
-
-        #l = create_session().query(User).select(order_by=[desc(users.c.name), asc(users.c.user_id)])
-        l = create_session().query(User).all()
-        #l = create_session().query(User).select(order_by=[])
-        #l = create_session().query(User).select(order_by=None)
+        assert "order by users.name desc" in str(create_session().query(User).statement).lower()
+        assert "order by" not in str(create_session().query(User).order_by(None).statement).lower()
+        assert "order by users.name asc" in str(create_session().query(User).order_by(User.name.asc()).statement).lower()
 
+        eq_(
+            create_session().query(User).all(),
+            [User(id=7, name=u'jack'), User(id=9, name=u'fred'), User(id=8, name=u'ed'), User(id=10, name=u'chuck')]
+        )
 
+        eq_(
+            create_session().query(User).order_by(User.name).all(),
+            [User(id=10, name=u'chuck'), User(id=8, name=u'ed'), User(id=9, name=u'fred'), User(id=7, name=u'jack')]
+        )
+        
     # 'Raises a "expression evaluation not supported" error at prepare time
     @testing.fails_on('firebird', 'FIXME: unknown')
     @testing.resolve_artifact_names