From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 5 Jul 2021 18:37:58 +0000 (-0400)
Subject: Add additional support to honor _proxy_key in Core selects
X-Git-Tag: rel_1_4_21~14^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4e1b2ee4cb345f499bdfbff6c393efbebd8e1e72;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Add additional support to honor _proxy_key in Core selects

Fixed ORM regression where ad-hoc label names generated for hybrid
properties and potentially other similar types of ORM-enabled expressions
would usually be propagated outwards through subqueries, allowing the name
to be retained in the final keys of the result set even when selecting from
subqueries. Additional state is now tracked in this case that isn't lost
when a hybrid is selected out of a Core select / subquery.

as we have removed things like column.label() from
ORM, since we now have to export the cols with the same names
as what we will render, experiment with giving a greater role
to the _proxy_key annotation so that a desired name can be
carried through more transarently.

Fixes: #6718
Change-Id: Icb313244c13ea1a8f58d3e05d07aa3e1039e15bf
---

diff --git a/doc/build/changelog/unreleased_14/6718.rst b/doc/build/changelog/unreleased_14/6718.rst
new file mode 100644
index 0000000000..05e20b927d
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6718.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: orm, regression
+    :tickets: 6718
+
+    Fixed ORM regression where ad-hoc label names generated for hybrid
+    properties and potentially other similar types of ORM-enabled expressions
+    would usually be propagated outwards through subqueries, allowing the name
+    to be retained in the final keys of the result set even when selecting from
+    subqueries. Additional state is now tracked in this case that isn't lost
+    when a hybrid is selected out of a Core select / subquery.
+
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index 78026efb1a..a6efac9cff 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -2890,7 +2890,7 @@ class _ORMColumnEntity(_ColumnEntity):
                 ezero._adapter if ezero.is_aliased_class else None,
             )
 
-        if column._annotations:
+        if column._annotations and not column._expression_label:
             # annotated columns perform more slowly in compiler and
             # result due to the __eq__() method, so use deannotated
             column = column._deannotate()
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 709106b6b9..f06aee74f7 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -887,6 +887,21 @@ class ColumnElement(
         else:
             return getattr(self, "name", "_no_label")
 
+    @util.memoized_property
+    def _expression_label(self):
+        """a suggested label to use in the case that the column has no name,
+        which should be used if possible as the explicit 'AS <label>'
+        where this expression would normally have an anon label.
+
+        """
+
+        if getattr(self, "name", None) is not None:
+            return None
+        elif self._annotations and "proxy_key" in self._annotations:
+            return self._annotations["proxy_key"]
+        else:
+            return None
+
     def _make_proxy(
         self, selectable, name=None, key=None, name_is_truncatable=False, **kw
     ):
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 557c443bf7..6ac9f0dbde 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -5815,14 +5815,17 @@ class Select(
                         repeated = c._anon_name_label in names
                         names[c._anon_name_label] = c
                         return (None, c, repeated)
+                    else:
+                        name = effective_name = c._label
                 elif getattr(c, "name", None) is None:
                     # this is a scalar_select().  need to improve this case
-                    repeated = c._anon_name_label in names
-                    names[c._anon_name_label] = c
-                    return (None, c, repeated)
-
-                if use_tablename_labels:
-                    name = effective_name = c._label
+                    expr_label = c._expression_label
+                    if expr_label is None:
+                        repeated = c._anon_name_label in names
+                        names[c._anon_name_label] = c
+                        return (None, c, repeated)
+                    else:
+                        name = effective_name = expr_label
                 else:
                     name = None
                     effective_name = c.name
diff --git a/test/ext/test_hybrid.py b/test/ext/test_hybrid.py
index 9085ccc96a..7597be86b5 100644
--- a/test/ext/test_hybrid.py
+++ b/test/ext/test_hybrid.py
@@ -5,6 +5,7 @@ from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import inspect
 from sqlalchemy import Integer
+from sqlalchemy import LABEL_STYLE_TABLENAME_PLUS_COL
 from sqlalchemy import literal_column
 from sqlalchemy import Numeric
 from sqlalchemy import select
@@ -244,6 +245,93 @@ class PropertyExpressionTest(fixtures.TestBase, AssertsCompiledSQL):
 
         return A, B
 
+    @testing.fixture
+    def _unnamed_expr_fixture(self):
+        Base = declarative_base()
+
+        class A(Base):
+            __tablename__ = "a"
+            id = Column(Integer, primary_key=True)
+            firstname = Column(String)
+            lastname = Column(String)
+
+            @hybrid.hybrid_property
+            def name(self):
+                return self.firstname + " " + self.lastname
+
+        return A
+
+    def test_labeling_for_unnamed(self, _unnamed_expr_fixture):
+        A = _unnamed_expr_fixture
+
+        stmt = select(A.id, A.name)
+        self.assert_compile(
+            stmt,
+            "SELECT a.id, a.firstname || :firstname_1 || a.lastname AS name "
+            "FROM a",
+        )
+
+        eq_(stmt.subquery().c.keys(), ["id", "name"])
+
+        self.assert_compile(
+            select(stmt.subquery()),
+            "SELECT anon_1.id, anon_1.name "
+            "FROM (SELECT a.id AS id, a.firstname || :firstname_1 || "
+            "a.lastname AS name FROM a) AS anon_1",
+        )
+
+    def test_labeling_for_unnamed_tablename_plus_col(
+        self, _unnamed_expr_fixture
+    ):
+        A = _unnamed_expr_fixture
+
+        stmt = select(A.id, A.name).set_label_style(
+            LABEL_STYLE_TABLENAME_PLUS_COL
+        )
+        # looks like legacy query
+        self.assert_compile(
+            stmt,
+            "SELECT a.id AS a_id, a.firstname || :firstname_1 || "
+            "a.lastname AS anon_1 FROM a",
+        )
+
+        # but no ORM translate...
+        eq_(stmt.subquery().c.keys(), ["a_id", "name"])
+
+        # then it comes out like this, not really sure if this is useful
+        self.assert_compile(
+            select(stmt.subquery()),
+            "SELECT anon_1.a_id, anon_1.anon_2 FROM (SELECT a.id AS a_id, "
+            "a.firstname || :firstname_1 || a.lastname AS anon_2 FROM a) "
+            "AS anon_1",
+        )
+
+    def test_labeling_for_unnamed_legacy(self, _unnamed_expr_fixture):
+        A = _unnamed_expr_fixture
+
+        sess = fixture_session()
+
+        stmt = sess.query(A.id, A.name)
+
+        # TABLENAME_PLUS_COL uses anon label right now, this is a little
+        # awkward looking, but loading.py translates
+        self.assert_compile(
+            stmt,
+            "SELECT a.id AS a_id, a.firstname || "
+            ":firstname_1 || a.lastname AS anon_1 FROM a",
+        )
+
+        # for the subquery, we lose the "ORM-ness" from the subquery
+        # so we have to carry it over using _proxy_key
+        eq_(stmt.subquery().c.keys(), ["id", "name"])
+
+        self.assert_compile(
+            sess.query(stmt.subquery()),
+            "SELECT anon_1.id AS anon_1_id, anon_1.name AS anon_1_name "
+            "FROM (SELECT a.id AS id, a.firstname || :firstname_1 || "
+            "a.lastname AS name FROM a) AS anon_1",
+        )
+
     def test_info(self):
         A = self._fixture()
         inspect(A).all_orm_descriptors.value.info["some key"] = "some value"