From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 7 Apr 2021 14:26:31 +0000 (-0400)
Subject: Check for hybrid's attribute name and support no name
X-Git-Tag: rel_1_4_7~17^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=475a1ada5d3ac88f12080ef8672a8fda70c7e76e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Check for hybrid's attribute name and support no name

Fixed regression where the ORM compilation scheme would assume the function
name of a hybrid property would be the same as the attribute name in such a
way that an ``AttributeError`` would be raised, when it would attempt to
determine the correct name for each element in a result tuple. A similar
issue exists in 1.3 but only impacts the names of tuple rows. The fix here
adds a check that the hybrid's function name is actually present in the
``__dict__`` of the class or its superclasses before assigning this name;
otherwise, the hybrid is considered to be "unnamed" and ORM result tuples
will use the naming scheme of the underlying expression.

Fixes: #6215
Change-Id: I584c0c05efec957f4dcaccf5df371399a57dffe9
---

diff --git a/doc/build/changelog/unreleased_14/6215.rst b/doc/build/changelog/unreleased_14/6215.rst
new file mode 100644
index 0000000000..c2ba306a41
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6215.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, regression, orm
+    :tickets: 6215
+
+    Fixed regression where the ORM compilation scheme would assume the function
+    name of a hybrid property would be the same as the attribute name in such a
+    way that an ``AttributeError`` would be raised, when it would attempt to
+    determine the correct name for each element in a result tuple. A similar
+    issue exists in 1.3 but only impacts the names of tuple rows. The fix here
+    adds a check that the hybrid's function name is actually present in the
+    ``__dict__`` of the class or its superclasses before assigning this name;
+    otherwise, the hybrid is considered to be "unnamed" and ORM result tuples
+    will use the naming scheme of the underlying expression.
diff --git a/lib/sqlalchemy/ext/hybrid.py b/lib/sqlalchemy/ext/hybrid.py
index 5fcb4fac0c..378e6c18b6 100644
--- a/lib/sqlalchemy/ext/hybrid.py
+++ b/lib/sqlalchemy/ext/hybrid.py
@@ -263,6 +263,34 @@ is supported, for more complex SET expressions it will usually be necessary
 to use either the "fetch" or False synchronization strategy as illustrated
 above.
 
+.. note:: For ORM bulk updates to work with hybrids, the function name
+   of the hybrid must match that of how it is accessed.    Something
+   like this wouldn't work::
+
+        class Interval(object):
+            # ...
+
+            def _get(self):
+                return self.end - self.start
+
+            def _set(self, value):
+                self.end = self.start + value
+
+            def _update_expr(cls, value):
+                return [
+                    (cls.end, cls.start + value)
+                ]
+
+            length = hybrid_property(
+                fget=_get, fset=_set, update_expr=_update_expr
+            )
+
+    The Python descriptor protocol does not provide any reliable way for
+    a descriptor to know what attribute name it was accessed as, and
+    the UPDATE scheme currently relies upon being able to access the
+    attribute from an instance by name in order to perform the instance
+    synchronization step.
+
 .. versionadded:: 1.2 added support for bulk updates to hybrid properties.
 
 Working with Relationships
@@ -1098,9 +1126,20 @@ class hybrid_property(interfaces.InspectionAttrInfo):
         proxy_attr = attributes.create_proxied_attribute(self)
 
         def expr_comparator(owner):
+            # because this is the descriptor protocol, we don't really know
+            # what our attribute name is.  so search for it through the
+            # MRO.
+            for lookup in owner.__mro__:
+                if self.__name__ in lookup.__dict__:
+                    if lookup.__dict__[self.__name__] is self:
+                        name = self.__name__
+                        break
+            else:
+                name = attributes.NO_KEY
+
             return proxy_attr(
                 owner,
-                self.__name__,
+                name,
                 self,
                 comparator(owner),
                 doc=comparator.__doc__ or self.__doc__,
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 610ee2726b..d1ed17f1a8 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -54,6 +54,13 @@ from ..sql import traversals
 from ..sql import visitors
 
 
+class NoKey(str):
+    pass
+
+
+NO_KEY = NoKey("no name")
+
+
 @inspection._self_inspects
 class QueryableAttribute(
     interfaces._MappedAttribute,
@@ -214,10 +221,15 @@ class QueryableAttribute(
         subclass representing a column expression.
 
         """
+        if self.key is NO_KEY:
+            annotations = {"entity_namespace": self._entity_namespace}
+        else:
+            annotations = {
+                "proxy_key": self.key,
+                "entity_namespace": self._entity_namespace,
+            }
 
-        return self.comparator.__clause_element__()._annotate(
-            {"proxy_key": self.key, "entity_namespace": self._entity_namespace}
-        )
+        return self.comparator.__clause_element__()._annotate(annotations)
 
     @property
     def _entity_namespace(self):
diff --git a/test/ext/test_hybrid.py b/test/ext/test_hybrid.py
index 3bab7db934..8be8023475 100644
--- a/test/ext/test_hybrid.py
+++ b/test/ext/test_hybrid.py
@@ -5,6 +5,7 @@ from sqlalchemy import func
 from sqlalchemy import inspect
 from sqlalchemy import Integer
 from sqlalchemy import Numeric
+from sqlalchemy import select
 from sqlalchemy import String
 from sqlalchemy.ext import hybrid
 from sqlalchemy.ext.declarative import declarative_base
@@ -97,6 +98,64 @@ class PropertyComparatorTest(fixtures.TestBase, AssertsCompiledSQL):
         A = self._fixture()
         eq_(A.value.__doc__, "This is a docstring")
 
+    def test_no_name_one(self):
+        """test :ticket:`6215`"""
+
+        Base = declarative_base()
+
+        class A(Base):
+            __tablename__ = "a"
+            id = Column(Integer, primary_key=True)
+            name = Column(String(50))
+
+            @hybrid.hybrid_property
+            def same_name(self):
+                return self.id
+
+            def name1(self):
+                return self.id
+
+            different_name = hybrid.hybrid_property(name1)
+
+            no_name = hybrid.hybrid_property(lambda self: self.name)
+
+        stmt = select(A.same_name, A.different_name, A.no_name)
+        compiled = stmt.compile()
+
+        eq_(
+            [ent._label_name for ent in compiled.compile_state._entities],
+            ["same_name", "id", "name"],
+        )
+
+    def test_no_name_two(self):
+        """test :ticket:`6215`"""
+        Base = declarative_base()
+
+        class SomeMixin(object):
+            @hybrid.hybrid_property
+            def same_name(self):
+                return self.id
+
+            def name1(self):
+                return self.id
+
+            different_name = hybrid.hybrid_property(name1)
+
+            no_name = hybrid.hybrid_property(lambda self: self.name)
+
+        class A(SomeMixin, Base):
+            __tablename__ = "a"
+            id = Column(Integer, primary_key=True)
+            name = Column(String(50))
+
+        stmt = select(A.same_name, A.different_name, A.no_name)
+        compiled = stmt.compile()
+
+        eq_(
+            [ent._label_name for ent in compiled.compile_state._entities],
+            ["same_name", "id", "name"],
+        )
+
 
 class PropertyExpressionTest(fixtures.TestBase, AssertsCompiledSQL):
     __dialect__ = "default"