diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-07-05 14:37:58 -0400 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2021-07-05 15:57:55 -0400 |
| commit | 4e1b2ee4cb345f499bdfbff6c393efbebd8e1e72 (patch) | |
| tree | 43d550e176570aebdd42c9d1662f1fdee2d0df63 | |
| parent | b920869ef54d05e73e2a980b73647d6050ffeb9d (diff) | |
| download | sqlalchemy-4e1b2ee4cb345f499bdfbff6c393efbebd8e1e72.tar.gz | |
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
| -rw-r--r-- | doc/build/changelog/unreleased_14/6718.rst | 11 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/context.py | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/elements.py | 15 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/selectable.py | 15 | ||||
| -rw-r--r-- | test/ext/test_hybrid.py | 88 |
5 files changed, 124 insertions, 7 deletions
diff --git a/doc/build/changelog/unreleased_14/6718.rst b/doc/build/changelog/unreleased_14/6718.rst new file mode 100644 index 000000000..05e20b927 --- /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 78026efb1..a6efac9cf 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 709106b6b..f06aee74f 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 557c443bf..6ac9f0dbd 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 9085ccc96..7597be86b 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" |
