diff options
| -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" |