From: Mike Bayer Date: Fri, 6 Jan 2017 22:02:32 +0000 (-0500) Subject: Tighten rules for order_by(Label) resolution X-Git-Tag: rel_1_1_5~17^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b489db89970b1fcec38a7c3772960ed3291a2ed;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Tighten rules for order_by(Label) resolution - Fixed bug originally introduced in 0.9 via :ticket:`1068` where order_by() would order by the label name based on name alone, that is, even if the labeled expression were not at all the same expression otherwise present, implicitly or explicitly, in the selectable. The logic that orders by label now ensures that the labeled expression is related to the one that resolves to that name before ordering by the label name; additionally, the name has to resolve to an actual label explicit in the expression elsewhere, not just a column name. This logic is carefully kept separate from the order by(textual name) feature that has a slightly different purpose. Change-Id: I44fc36dab34380cc238c1e79ecbe23f1628d588a Fixes: #3882 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 3fed21b170..1554987c10 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,21 @@ .. changelog:: :version: 1.1.5 + .. change:: 3882 + :tags: bug, sql + :tikets: 3882 + + Fixed bug originally introduced in 0.9 via :ticket:`1068` where + order_by() would order by the label name based on name + alone, that is, even if the labeled expression were not at all the same + expression otherwise present, implicitly or explicitly, in the + selectable. The logic that orders by label now ensures that the + labeled expression is related to the one that resolves to that name + before ordering by the label name; additionally, the name has to + resolve to an actual label explicit in the expression elsewhere, not + just a column name. This logic is carefully kept separate from the + order by(textual name) feature that has a slightly different purpose. + .. change:: try_finally_for_noautoflush :tags: bug, orm diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index b8c897cb90..f6f9fa45dc 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -580,11 +580,11 @@ class SQLCompiler(Compiled): if self.stack and self.dialect.supports_simple_order_by_label: selectable = self.stack[-1]['selectable'] - with_cols, only_froms = selectable._label_resolve_dict + with_cols, only_froms, only_cols = selectable._label_resolve_dict if within_columns_clause: resolve_dict = only_froms else: - resolve_dict = with_cols + resolve_dict = only_cols # this can be None in the case that a _label_reference() # were subject to a replacement operation, in which case @@ -593,11 +593,11 @@ class SQLCompiler(Compiled): order_by_elem = element.element._order_by_label_element if order_by_elem is not None and order_by_elem.name in \ - resolve_dict: - + resolve_dict and \ + order_by_elem.shares_lineage( + resolve_dict[order_by_elem.name]): kwargs['render_label_as_label'] = \ element.element._order_by_label_element - return self.process( element.element, within_columns_clause=within_columns_clause, **kwargs) @@ -611,7 +611,7 @@ class SQLCompiler(Compiled): ) selectable = self.stack[-1]['selectable'] - with_cols, only_froms = selectable._label_resolve_dict + with_cols, only_froms, only_cols = selectable._label_resolve_dict try: if within_columns_clause: col = only_froms[element.element] diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index ca4183e3e9..62c8205dc4 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -2275,7 +2275,7 @@ class CompoundSelect(GenerativeSelect): d = dict( (c.key, c) for c in self.c ) - return d, d + return d, d, d @classmethod def _create_union(cls, *selects, **kwargs): @@ -2948,10 +2948,11 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): only_froms = dict( (c.key, c) for c in _select_iterables(self.froms) if c._allow_label_resolve) + only_cols = with_cols.copy() for key, value in only_froms.items(): with_cols.setdefault(key, value) - return with_cols, only_froms + return with_cols, only_froms, only_cols def is_derived_from(self, fromclause): if self in fromclause._cloned_set: diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index a85786bed8..38ca09c0a3 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -952,6 +952,56 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): 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')) + ), + "SELECT mytable.myid + :myid_1 AS foo, " + "somefunc(mytable.name) AS bar FROM mytable " + "ORDER BY mytable.myid, mytable.name DESC", + 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')) + ), + "SELECT mytable.myid + :myid_1 AS foo, " + "somefunc(mytable.name) AS bar FROM mytable " + "ORDER BY mytable_1.name DESC", + 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 mytable.myid + :myid_1 AS foo, " + "somefunc(mytable.name) AS bar FROM mytable " + "ORDER BY bar DESC", + 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): lab1 = (table1.c.myid + 12).label('foo') lab2 = func.somefunc(table1.c.name).label('bar')