From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 18 Mar 2021 23:34:32 +0000 (-0400)
Subject: Ensure ClauseAdapter treats FunctionElement as a ColumnElement
X-Git-Tag: rel_1_4_2~8^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1c999c01d595b74fe178d9bdbff34fd8939a283;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Ensure ClauseAdapter treats FunctionElement as a ColumnElement

Fixed regression where use of an unnamed SQL expression such as a SQL
function would raise a column targeting error if the query itself were
using joinedload for an entity and was also being wrapped in a subquery by
the joinedload eager loading process.

Fixes: #6086
Change-Id: I22cf4d6974685267c4f903bd7639be8271c6c1ef
---

diff --git a/doc/build/changelog/unreleased_14/6086.rst b/doc/build/changelog/unreleased_14/6086.rst
new file mode 100644
index 0000000000..5510883474
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6086.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, regression, orm
+    :tickets: 6086
+
+    Fixed regression where use of an unnamed SQL expression such as a SQL
+    function would raise a column targeting error if the query itself were
+    using joinedload for an entity and was also being wrapped in a subquery by
+    the joinedload eager loading process.
+
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index 9cfa0fdc6d..e440ee1614 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -2707,7 +2707,9 @@ class _ORMColumnEntity(_ColumnEntity):
             compile_state._entities.append(self)
 
         compile_state._has_orm_entities = True
+
         self.column = column
+
         self._fetch_column = self._row_processor = None
 
         self._extra_entities = (self.expr, self.column)
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 5f35de5f00..4300d8a298 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -843,8 +843,13 @@ class ClauseAdapter(visitors.ReplacingExternalTraversal):
             newcol = self.selectable.exported_columns.get(col.name)
         return newcol
 
+    @util.preload_module("sqlalchemy.sql.functions")
     def replace(self, col):
-        if isinstance(col, FromClause):
+        functions = util.preloaded.sql_functions
+
+        if isinstance(col, FromClause) and not isinstance(
+            col, functions.FunctionElement
+        ):
             if self.selectable.is_derived_from(col):
                 return self.selectable
             elif isinstance(col, Alias) and isinstance(
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index f62d3af331..289f8e8b1e 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -488,6 +488,37 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
         self.assert_sql_count(testing.db, go, 1)
 
+    def test_aliased_stmt_includes_unnamed_fn(self):
+        User, Address = self.classes("User", "Address")
+        users, addresses = self.tables("users", "addresses")
+        mapper(
+            User,
+            users,
+            properties={"addresses": relationship(Address, lazy="joined")},
+        )
+        mapper(Address, addresses)
+
+        s = fixture_session()
+
+        # issue #6086
+        # statement wrapped in a subquery by limit() and group_by()
+        # func.count() is unlabeled (in 1.3 the _ColumnEntity would label it,
+        # in the ORM layer,  hence there was no problem here).
+        # the _ColumnEntity needs to adapt func.count(User.id) to the anon
+        # count_1 label on the outside, corresponding_column can do it.
+        # but ClauseAdapter has to treat the FunctionElement as a ColumnElement
+        # whereas previously it was treating it as a FromClause (and
+        # FunctionElement should really not even be a FromClause but there's
+        # legacy baggage on that)
+        q = (
+            s.query(User, func.count(User.id))
+            .order_by(User.id)
+            .group_by(User.id)
+            .limit(1)
+        )
+
+        eq_(q.first(), (User(id=7), 1))
+
     def test_options_pathing(self):
         (
             users,
diff --git a/test/sql/test_external_traversal.py b/test/sql/test_external_traversal.py
index bc60137913..b7e58dad9e 100644
--- a/test/sql/test_external_traversal.py
+++ b/test/sql/test_external_traversal.py
@@ -1338,6 +1338,40 @@ class ClauseAdapterTest(fixtures.TestBase, AssertsCompiledSQL):
             "AS anon_1 FROM table1 AS t1alias",
         )
 
+    def test_adapt_select_w_unlabeled_fn(self):
+
+        expr = func.count(t1.c.col1)
+        stmt = select(t1, expr)
+
+        self.assert_compile(
+            stmt,
+            "SELECT table1.col1, table1.col2, table1.col3, "
+            "count(table1.col1) AS count_1 FROM table1",
+        )
+
+        stmt2 = select(stmt.subquery())
+
+        self.assert_compile(
+            stmt2,
+            "SELECT anon_1.col1, anon_1.col2, anon_1.col3, anon_1.count_1 "
+            "FROM (SELECT table1.col1 AS col1, table1.col2 AS col2, "
+            "table1.col3 AS col3, count(table1.col1) AS count_1 "
+            "FROM table1) AS anon_1",
+        )
+
+        is_(
+            stmt2.selected_columns[3],
+            stmt2.selected_columns.corresponding_column(expr),
+        )
+
+        is_(
+            sql_util.ClauseAdapter(stmt2).replace(expr),
+            stmt2.selected_columns[3],
+        )
+
+        column_adapter = sql_util.ColumnAdapter(stmt2)
+        is_(column_adapter.columns[expr], stmt2.selected_columns[3])
+
     def test_correlate_except_on_clone(self):
         # test [ticket:4537]'s issue