]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Add additional support to honor _proxy_key in Core selects
authorMike Bayer <mike_mp@zzzcomputing.com>
Mon, 5 Jul 2021 18:37:58 +0000 (14:37 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Mon, 5 Jul 2021 19:57:55 +0000 (15:57 -0400)
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

doc/build/changelog/unreleased_14/6718.rst [new file with mode: 0644]
lib/sqlalchemy/orm/context.py
lib/sqlalchemy/sql/elements.py
lib/sqlalchemy/sql/selectable.py
test/ext/test_hybrid.py

diff --git a/doc/build/changelog/unreleased_14/6718.rst b/doc/build/changelog/unreleased_14/6718.rst
new file mode 100644 (file)
index 0000000..05e20b9
--- /dev/null
@@ -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.
+
index 78026efb1aad4cb3eb1d26428ea9ea8ef90d1a34..a6efac9cff69be965824a11979367b81ef16d5e2 100644 (file)
@@ -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()
index 709106b6b9ba7073745e3d4a16244de64506ffa2..f06aee74f74c5cef12f8fecc544e1b97e900c8a6 100644 (file)
@@ -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
     ):
index 557c443bf7cfdbd2fe9271df887d3ed5d1b0bf2b..6ac9f0dbde5f50fad23b94706c15a915d0adea79 100644 (file)
@@ -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
index 9085ccc96a0f4268a94ffe0760be4a55ba6a9523..7597be86b55939ee7744151e573dec79b3fad348 100644 (file)
@@ -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"