From c0af2ee6c992cf6574d8eea052d7cd3b0fabd337 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 2 Sep 2014 10:49:46 -0400 Subject: [PATCH] - add logic to compiler such that if stack is empty, we just stringify a _label_reference() as is. - add .key to _label_reference(), so that when _make_proxy() is called, we don't call str() on it anyway. - add a test to exercise Query's behavior of adding all the order_by expressions to the columns list of the select, assert that things work out when we have a _label_reference there, that it gets sucked into the columns list and spit out on the other side, it's referred to appropriately, etc. _label_reference() could theoretically be resolved at the point we iterate _raw_columns() but it's better to just let things work as they already do (except nicer, since we get "tablename.colname" instead of just "somename" in the columns list) so that we aren't adding a ton of overhead to _columns_plus_names in the common case. --- lib/sqlalchemy/sql/compiler.py | 8 ++++++- lib/sqlalchemy/sql/elements.py | 6 ++++- lib/sqlalchemy/sql/selectable.py | 1 + test/orm/test_query.py | 40 +++++++++++++++++++++++++++++++- test/sql/test_text.py | 5 ++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index e4597dcd83..23e5456a7d 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -495,6 +495,12 @@ class SQLCompiler(Compiled): return "(" + grouping.element._compiler_dispatch(self, **kwargs) + ")" def visit_label_reference(self, element, **kwargs): + if not self.stack: + # compiling the element outside of the context of a SELECT + return self.process( + element._text_clause + ) + selectable = self.stack[-1]['selectable'] try: col = selectable._inner_column_dict[element.text] @@ -504,7 +510,7 @@ class SQLCompiler(Compiled): "Can't resolve label reference %r; converting to text()", util.ellipses_string(element.text)) return self.process( - elements.TextClause._create_text(element.text) + element._text_clause ) else: kwargs['render_label_as_label'] = col diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 0ea05fa0ec..984cfe0eea 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -2292,7 +2292,11 @@ class _label_reference(ColumnElement): __visit_name__ = 'label_reference' def __init__(self, text): - self.text = text + self.text = self.key = text + + @util.memoized_property + def _text_clause(self): + return TextClause._create_text(self.text) class UnaryExpression(ColumnElement): diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index cf2c213d28..a494939959 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -2976,6 +2976,7 @@ class Select(HasPrefixes, GenerativeSelect): def name_for_col(c): if c._columns_clause_label is None: return (None, c) + name = c._columns_clause_label if name in names: name = c.anon_label diff --git a/test/orm/test_query.py b/test/orm/test_query.py index c0e9f9e1c4..cb67057e43 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -2276,7 +2276,7 @@ class HintsTest(QueryTest, AssertsCompiledSQL): ) -class TextTest(QueryTest): +class TextTest(QueryTest, AssertsCompiledSQL): def test_fulltext(self): User = self.classes.User @@ -2380,6 +2380,44 @@ class TextTest(QueryTest): [User(id=7), User(id=8), User(id=9), User(id=10)] ) + def test_order_by_w_eager(self): + User = self.classes.User + Address = self.classes.Address + s = create_session() + + # here, we are seeing how Query has to take the order by expressions + # of the query and then add them to the columns list, so that the + # outer subquery can order by that same label. With the anonymous + # label, our column gets sucked up and restated again in the + # inner columns list! + # we could try to play games with making this "smarter" but it + # would add permanent overhead to Select._columns_plus_names, + # since that's where references would need to be resolved. + # so as it is, this query takes the _label_reference and makes a + # full blown proxy and all the rest of it. + self.assert_compile( + s.query(User).options(joinedload("addresses")). + order_by(desc("name")).limit(1), + "SELECT anon_1.users_id AS anon_1_users_id, " + "anon_1.users_name AS anon_1_users_name, " + "anon_1.anon_2 AS anon_1_anon_2, " + "addresses_1.id AS addresses_1_id, " + "addresses_1.user_id AS addresses_1_user_id, " + "addresses_1.email_address AS addresses_1_email_address " + "FROM (SELECT users.id AS users_id, users.name AS users_name, " + "users.name AS anon_2 FROM users ORDER BY users.name " + "DESC LIMIT ? OFFSET ?) AS anon_1 " + "LEFT OUTER JOIN addresses AS addresses_1 " + "ON anon_1.users_id = addresses_1.user_id " + "ORDER BY anon_1.anon_2 DESC, addresses_1.id" + ) + + eq_( + s.query(User).options(joinedload("addresses")). + order_by(desc("name")).first(), + User(name='jack', addresses=[Address()]) + ) + class TextWarningTest(QueryTest, AssertsCompiledSQL): def _test(self, fn, arg, offending_clause, expected): diff --git a/test/sql/test_text.py b/test/sql/test_text.py index 182c63624a..e84a2907c7 100644 --- a/test/sql/test_text.py +++ b/test/sql/test_text.py @@ -674,3 +674,8 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL): "SELECT foo(:foo_1) AS x UNION SELECT foo(:foo_2) AS y ORDER BY x" ) + def test_standalone_units_stringable(self): + self.assert_compile( + desc("somelabel"), + "somelabel DESC" + ) -- 2.47.3