diff options
| -rw-r--r-- | doc/build/changelog/changelog_11.rst | 15 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/compiler.py | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/selectable.py | 5 | ||||
| -rw-r--r-- | test/sql/test_compiler.py | 50 |
4 files changed, 74 insertions, 8 deletions
diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 3fed21b17..1554987c1 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(<some Label()>) 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 b8c897cb9..f6f9fa45d 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 ca4183e3e..62c8205dc 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 a85786bed..38ca09c0a 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') |