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 + 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: - 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 + 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()