From: Mike Bayer Date: Wed, 10 Jul 2024 14:32:44 +0000 (-0400) Subject: include nulls_first, nulls_last in order_by_label_element X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96f1172812f858fead45cdc7874abac76f45b339;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git include nulls_first, nulls_last in order_by_label_element Fixed bug where the :meth:`.Operators.nulls_first()` and :meth:`.Operators.nulls_last()` modifiers would not be treated the same way as :meth:`.Operators.desc()` and :meth:`.Operators.asc()` when determining if an ORDER BY should be against a label name already in the statement. All four modifiers are now treated the same within ORDER BY. Fixes: #11592 Change-Id: I1de1aff679c56af1abfdfd07f9bcbc45ecc5a8cc --- diff --git a/doc/build/changelog/unreleased_20/11592.rst b/doc/build/changelog/unreleased_20/11592.rst new file mode 100644 index 0000000000..616eb1e286 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11592.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, sql + :tickets: 11592 + + Fixed bug where the :meth:`.Operators.nulls_first()` and + :meth:`.Operators.nulls_last()` modifiers would not be treated the same way + as :meth:`.Operators.desc()` and :meth:`.Operators.asc()` when determining + if an ORDER BY should be against a label name already in the statement. All + four modifiers are now treated the same within ORDER BY. diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 56b937726e..3271acd60d 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -3699,7 +3699,7 @@ class UnaryExpression(ColumnElement[_T]): @property def _order_by_label_element(self) -> Optional[Label[Any]]: - if self.modifier in (operators.desc_op, operators.asc_op): + if operators.is_order_by_modifier(self.modifier): return self.element._order_by_label_element else: return None diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py index a5390ad6d0..65b94f32b5 100644 --- a/lib/sqlalchemy/sql/operators.py +++ b/lib/sqlalchemy/sql/operators.py @@ -2474,6 +2474,12 @@ def is_associative(op: OperatorType) -> bool: return op in _associative +def is_order_by_modifier(op: Optional[OperatorType]) -> bool: + return op in _order_by_modifier + + +_order_by_modifier = {desc_op, asc_op, nulls_first_op, nulls_last_op} + _natural_self_precedent = _associative.union( [getitem, json_getitem_op, json_path_getitem_op] ) diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 9d9f69bdb9..3e8fca59a8 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -44,6 +44,10 @@ from sqlalchemy import literal_column from sqlalchemy import MetaData from sqlalchemy import not_ from sqlalchemy import null +from sqlalchemy import nulls_first +from sqlalchemy import nulls_last +from sqlalchemy import nullsfirst +from sqlalchemy import nullslast from sqlalchemy import Numeric from sqlalchemy import or_ from sqlalchemy import outerjoin @@ -1668,44 +1672,85 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): "foo || :param_1", ) - def test_order_by_labels_enabled(self): + def test_order_by_labels_enabled_negative_cases(self): + """test order_by_labels enabled but the cases where we expect + ORDER BY the expression without the label name""" + lab1 = (table1.c.myid + 12).label("foo") lab2 = func.somefunc(table1.c.name).label("bar") dialect = default.DefaultDialect() + # binary expressions render as the expression without labels self.assert_compile( - select(lab1, lab2).order_by(lab1, desc(lab2)), + select(lab1, lab2).order_by(lab1 + "test"), "SELECT mytable.myid + :myid_1 AS foo, " "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY foo, bar DESC", + "ORDER BY mytable.myid + :myid_1 + :param_1", dialect=dialect, ) - # the function embedded label renders as the function + # labels within functions in the columns clause render + # with the expression self.assert_compile( - select(lab1, lab2).order_by(func.hoho(lab1), desc(lab2)), + select(lab1, func.foo(lab1)).order_by(lab1, func.foo(lab1)), "SELECT mytable.myid + :myid_1 AS foo, " - "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY hoho(mytable.myid + :myid_1), bar DESC", + "foo(mytable.myid + :myid_1) AS foo_1 FROM mytable " + "ORDER BY foo, foo(mytable.myid + :myid_1)", dialect=dialect, ) - # binary expressions render as the expression without labels + # here, 'name' is implicitly available, but w/ #3882 we don't + # want to render a name that isn't specifically a Label elsewhere + # in the query self.assert_compile( - select(lab1, lab2).order_by(lab1 + "test"), + select(table1.c.myid).order_by(table1.c.name.label("name")), + "SELECT mytable.myid FROM mytable ORDER BY mytable.name", + ) + + # as well as if it doesn't match + self.assert_compile( + select(table1.c.myid).order_by( + func.lower(table1.c.name).label("name") + ), + "SELECT mytable.myid FROM mytable ORDER BY lower(mytable.name)", + ) + + @testing.combinations( + (desc, "DESC"), + (asc, "ASC"), + (nulls_first, "NULLS FIRST"), + (nulls_last, "NULLS LAST"), + (nullsfirst, "NULLS FIRST"), + (nullslast, "NULLS LAST"), + (lambda c: c.desc().nulls_last(), "DESC NULLS LAST"), + (lambda c: c.desc().nullslast(), "DESC NULLS LAST"), + (lambda c: c.nulls_first().asc(), "NULLS FIRST ASC"), + ) + def test_order_by_labels_enabled(self, operator, expected): + """test positive cases with order_by_labels enabled. this is + multipled out to all the ORDER BY modifier operators + (see #11592) + + + """ + lab1 = (table1.c.myid + 12).label("foo") + lab2 = func.somefunc(table1.c.name).label("bar") + dialect = default.DefaultDialect() + + self.assert_compile( + select(lab1, lab2).order_by(lab1, operator(lab2)), "SELECT mytable.myid + :myid_1 AS foo, " "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY mytable.myid + :myid_1 + :param_1", + f"ORDER BY foo, bar {expected}", dialect=dialect, ) - # labels within functions in the columns clause render - # with the expression + # the function embedded label renders as the function self.assert_compile( - select(lab1, func.foo(lab1)).order_by(lab1, func.foo(lab1)), + select(lab1, lab2).order_by(func.hoho(lab1), operator(lab2)), "SELECT mytable.myid + :myid_1 AS foo, " - "foo(mytable.myid + :myid_1) AS foo_1 FROM mytable " - "ORDER BY foo, foo(mytable.myid + :myid_1)", + "somefunc(mytable.name) AS bar FROM mytable " + f"ORDER BY hoho(mytable.myid + :myid_1), bar {expected}", dialect=dialect, ) @@ -1713,62 +1758,49 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): ly = (func.lower(table1.c.name) + table1.c.description).label("ly") self.assert_compile( - select(lx, ly).order_by(lx, ly.desc()), + select(lx, ly).order_by(lx, operator(ly)), "SELECT mytable.myid + mytable.myid AS lx, " "lower(mytable.name) || mytable.description AS ly " - "FROM mytable ORDER BY lx, ly DESC", + f"FROM mytable ORDER BY lx, ly {expected}", dialect=dialect, ) # expression isn't actually the same thing (even though label is) self.assert_compile( select(lab1, lab2).order_by( - table1.c.myid.label("foo"), desc(table1.c.name.label("bar")) + table1.c.myid.label("foo"), + operator(table1.c.name.label("bar")), ), "SELECT mytable.myid + :myid_1 AS foo, " "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY mytable.myid, mytable.name DESC", + f"ORDER BY mytable.myid, mytable.name {expected}", dialect=dialect, ) # it's also an exact match, not aliased etc. self.assert_compile( select(lab1, lab2).order_by( - desc(table1.alias().c.name.label("bar")) + operator(table1.alias().c.name.label("bar")) ), "SELECT mytable.myid + :myid_1 AS foo, " "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY mytable_1.name DESC", + f"ORDER BY mytable_1.name {expected}", dialect=dialect, ) # but! it's based on lineage lab2_lineage = lab2.element._clone() self.assert_compile( - select(lab1, lab2).order_by(desc(lab2_lineage.label("bar"))), + select(lab1, lab2).order_by(operator(lab2_lineage.label("bar"))), "SELECT mytable.myid + :myid_1 AS foo, " "somefunc(mytable.name) AS bar FROM mytable " - "ORDER BY bar DESC", + f"ORDER BY bar {expected}", dialect=dialect, ) - # here, 'name' is implicitly available, but w/ #3882 we don't - # want to render a name that isn't specifically a Label elsewhere - # in the query - self.assert_compile( - select(table1.c.myid).order_by(table1.c.name.label("name")), - "SELECT mytable.myid FROM mytable ORDER BY mytable.name", - ) - - # as well as if it doesn't match - self.assert_compile( - select(table1.c.myid).order_by( - func.lower(table1.c.name).label("name") - ), - "SELECT mytable.myid FROM mytable ORDER BY lower(mytable.name)", - ) - def test_order_by_labels_disabled(self): + """test when the order_by_labels feature is disabled entirely""" + lab1 = (table1.c.myid + 12).label("foo") lab2 = func.somefunc(table1.c.name).label("bar") dialect = default.DefaultDialect()