From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 28 Apr 2021 13:56:15 +0000 (-0400)
Subject: add optional proxy_class to track w/ proxy_key
X-Git-Tag: rel_1_4_12~13^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=41ac0c7187daed54b0ba44b2287b6679d95d2caa;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

add optional proxy_class to track w/ proxy_key

Fixed regression in ORM where using hybrid property to indicate an
expression from a different entity would confuse the column-labeling logic
in the ORM and attempt to derive the name of the hybrid from that other
class, leading to an attribute error. The owning class of the hybrid
attribute is now tracked along with the name.

Fixes: #6386
Change-Id: Ica9497ea34fef799d6265de44104c1f3f3b30232
---

diff --git a/doc/build/changelog/unreleased_14/6386.rst b/doc/build/changelog/unreleased_14/6386.rst
new file mode 100644
index 0000000000..d61a2cc482
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6386.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: ext, bug, regression
+    :tickets: 6386
+
+    Fixed regression in ORM where using hybrid property to indicate an
+    expression from a different entity would confuse the column-labeling logic
+    in the ORM and attempt to derive the name of the hybrid from that other
+    class, leading to an attribute error. The owning class of the hybrid
+    attribute is now tracked along with the name.
\ No newline at end of file
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 0c7bc4cf0f..b8974196cf 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -227,6 +227,7 @@ class QueryableAttribute(
         else:
             annotations = {
                 "proxy_key": self.key,
+                "proxy_owner": self.class_,
                 "entity_namespace": self._entity_namespace,
             }
 
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index 6cdad9f417..042acc9c50 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -2691,8 +2691,9 @@ class _ORMColumnEntity(_ColumnEntity):
         # within internal loaders.
 
         orm_key = annotations.get("proxy_key", None)
+        proxy_owner = annotations.get("proxy_owner", _entity.entity)
         if orm_key:
-            self.expr = getattr(_entity.entity, orm_key)
+            self.expr = getattr(proxy_owner, orm_key)
             self.translate_raw_column = False
         else:
             # if orm_key is not present, that means this is an ad-hoc
diff --git a/test/ext/test_hybrid.py b/test/ext/test_hybrid.py
index c9373f9aa0..ee991782ef 100644
--- a/test/ext/test_hybrid.py
+++ b/test/ext/test_hybrid.py
@@ -492,6 +492,69 @@ class PropertyMirrorTest(fixtures.TestBase, AssertsCompiledSQL):
 
         return A
 
+    @testing.fixture
+    def _name_mismatch_fixture(self):
+        Base = declarative_base()
+
+        class A(Base):
+            __tablename__ = "a"
+            id = Column(Integer, primary_key=True)
+            addresses = relationship("B")
+
+            @hybrid.hybrid_property
+            def some_email(self):
+                if self.addresses:
+                    return self.addresses[0].email_address
+                else:
+                    return None
+
+            @some_email.expression
+            def some_email(cls):
+                return B.email_address
+
+        class B(Base):
+            __tablename__ = "b"
+            id = Column(Integer, primary_key=True)
+            aid = Column(ForeignKey("a.id"))
+            email_address = Column(String)
+
+        return A, B
+
+    def test_dont_assume_attr_key_is_present(self, _name_mismatch_fixture):
+        A, B = _name_mismatch_fixture
+        self.assert_compile(
+            select(A, A.some_email).join(A.addresses),
+            "SELECT a.id, b.email_address FROM a JOIN b ON a.id = b.aid",
+        )
+
+    def test_dont_assume_attr_key_is_present_ac(self, _name_mismatch_fixture):
+        A, B = _name_mismatch_fixture
+
+        ac = aliased(A)
+        self.assert_compile(
+            select(ac, ac.some_email).join(ac.addresses),
+            "SELECT a_1.id, b.email_address "
+            "FROM a AS a_1 JOIN b ON a_1.id = b.aid",
+        )
+
+    def test_filter_by_mismatched_col(self, _name_mismatch_fixture):
+        A, B = _name_mismatch_fixture
+        self.assert_compile(
+            select(A).filter_by(some_email="foo").join(A.addresses),
+            "SELECT a.id FROM a JOIN b ON a.id = b.aid "
+            "WHERE b.email_address = :email_address_1",
+        )
+
+    def test_aliased_mismatched_col(self, _name_mismatch_fixture):
+        A, B = _name_mismatch_fixture
+        sess = fixture_session()
+
+        # so what should this do ?   it's just a weird hybrid case
+        self.assert_compile(
+            sess.query(aliased(A).some_email),
+            "SELECT b.email_address AS b_email_address FROM b",
+        )
+
     def test_property(self):
         A = self._fixture()
 
diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py
index 8b298c6fba..c0829e9b3c 100644
--- a/test/orm/test_utils.py
+++ b/test/orm/test_utils.py
@@ -246,6 +246,7 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
                 "parententity": point_mapper,
                 "parentmapper": point_mapper,
                 "proxy_key": "x_alone",
+                "proxy_owner": Point,
             },
         )
         eq_(
@@ -255,6 +256,7 @@ class AliasedClassTest(fixtures.TestBase, AssertsCompiledSQL):
                 "parententity": point_mapper,
                 "parentmapper": point_mapper,
                 "proxy_key": "x",
+                "proxy_owner": Point,
             },
         )