Add QueryableAttribute._impl_uses_objects accessor for AssociationProxy

Fixed regression where new association proxy system was still not proxying
hybrid attributes when they made use of the ``@hybrid_property.expression``
decorator to return an alternate SQL expression, or when the hybrid
returned an arbitrary :class:`.PropComparator`, at the expression level.
This involved futher generalization of the heuristics used to detect the
type of object being proxied at the level of :class:`.QueryableAttribute`,
to better detect if the descriptor ultimately serves mapped classes or
column expressions.

Fixes: #4690
Change-Id: I5b5300661291c94a23de53bcf92d747701720aa1

diff --git a/doc/build/changelog/unreleased_13/4690.rst b/doc/build/changelog/unreleased_13/4690.rst
new file mode 100644 (file)
index 0000000..e9aa4be
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4690.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4690
+
+    Fixed regression where new association proxy system was still not proxying
+    hybrid attributes when they made use of the ``@hybrid_property.expression``
+    decorator to return an alternate SQL expression, or when the hybrid
+    returned an arbitrary :class:`.PropComparator`, at the expression level.
+    This involved futher generalization of the heuristics used to detect the
+    type of object being proxied at the level of :class:`.QueryableAttribute`,
+    to better detect if the descriptor ultimately serves mapped classes or
+    column expressions.

diff --git a/lib/sqlalchemy/ext/associationproxy.py b/lib/sqlalchemy/ext/associationproxy.py
index c725372c5c00e75758aa744090380a4d33fc9669..9a8294f3c776ff4116186320a505de182ed242da 100644 (file)
--- a/lib/sqlalchemy/ext/associationproxy.py
+++ b/lib/sqlalchemy/ext/associationproxy.py
@@ -391,15 +391,11 @@ class AssociationProxyInstance(object):
             )
 
         attr = getattr(target_class, value_attr)
-        if (
-            not hasattr(attr, "_is_internal_proxy")
-            or attr._is_internal_proxy
-            and not hasattr(attr, "impl")
-        ):
+        if not hasattr(attr, "_is_internal_proxy"):
             return AmbiguousAssociationProxyInstance(
                 parent, owning_class, target_class, value_attr
             )
-        is_object = attr.impl.uses_objects
+        is_object = attr._impl_uses_objects
         if is_object:
             return ObjectAssociationProxyInstance(
                 parent, owning_class, target_class, value_attr

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index f37928cc1ebd60011ed6683b2e3f98bb713582d7..674c57feac719a89b93d817cf654dd683f60aa45 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -104,6 +104,10 @@ class QueryableAttribute(
     def _supports_population(self):
         return self.impl.supports_population
 
+    @property
+    def _impl_uses_objects(self):
+        return self.impl.uses_objects
+
     def get_history(self, instance, passive=PASSIVE_OFF):
         return self.impl.get_history(
             instance_state(instance), instance_dict(instance), passive
@@ -309,6 +313,13 @@ def create_proxied_attribute(descriptor):
 
         _is_internal_proxy = True
 
+        @property
+        def _impl_uses_objects(self):
+            return (
+                self.original_property is not None
+                and getattr(self.class_, self.key).impl.uses_objects
+            )
+
         @property
         def property(self):
             return self.comparator.property

diff --git a/test/ext/test_associationproxy.py b/test/ext/test_associationproxy.py
index 683a84fcdfbf5f6ab79da32f606955b4b177e0f9..d515ccf6563c0f4bd456cc58d074745efdf5cd80 100644 (file)
--- a/test/ext/test_associationproxy.py
+++ b/test/ext/test_associationproxy.py
@@ -1,6 +1,7 @@
 import copy
 import pickle
 
+from sqlalchemy import cast
 from sqlalchemy import exc
 from sqlalchemy import ForeignKey
 from sqlalchemy import inspect
@@ -3141,7 +3142,9 @@ class ProxyOfSynonymTest(AssertsCompiledSQL, fixtures.DeclarativeMappedTest):
         )
 
 
-class ProxyHybridTest(fixtures.DeclarativeMappedTest):
+class ProxyHybridTest(fixtures.DeclarativeMappedTest, AssertsCompiledSQL):
+    __dialect__ = "default"
+
     @classmethod
     def setup_classes(cls):
         from sqlalchemy.ext.hybrid import hybrid_property
@@ -3185,9 +3188,30 @@ class ProxyHybridTest(fixtures.DeclarativeMappedTest):
             class value(PropComparator):
                 # comparator has no proxy __getattr__, so we can't
                 # get to impl to see what we ar proxying towards.
+                # as of #4690 we assume column-oriented proxying
                 def __init__(self, cls):
                     self.cls = cls
 
+            @hybrid_property
+            def well_behaved_w_expr(self):
+                return self.data
+
+            @well_behaved_w_expr.setter
+            def well_behaved_w_expr(self, value):
+                self.data = value
+
+            @well_behaved_w_expr.expression
+            def well_behaved_w_expr(cls):
+                return cast(cls.data, Integer)
+
+        class C(Base):
+            __tablename__ = "c"
+
+            id = Column(Integer, primary_key=True)
+            b_id = Column(ForeignKey("b.id"))
+            _b = relationship("B")
+            attr = association_proxy("_b", "well_behaved_w_expr")
+
     def test_get_ambiguous(self):
         A, B = self.classes("A", "B")
 
@@ -3230,18 +3254,29 @@ class ProxyHybridTest(fixtures.DeclarativeMappedTest):
 
         eq_(
             str(A.b_data),
-            "AmbiguousAssociationProxyInstance"
+            "ColumnAssociationProxyInstance"
             "(AssociationProxy('bs', 'value'))",
         )
 
-    def test_expr_ambiguous(self):
+    def test_comparator_ambiguous(self):
         A, B = self.classes("A", "B")
 
-        assert_raises_message(
-            AttributeError,
-            "Association proxy A.bs refers to an attribute "
-            "'value' that is not directly mapped",
-            A.b_data.any,
+        s = Session()
+        self.assert_compile(
+            s.query(A).filter(A.b_data.any()),
+            "SELECT a.id AS a_id FROM a WHERE EXISTS "
+            "(SELECT 1 FROM b WHERE a.id = b.aid)",
+        )
+
+    def test_explicit_expr(self):
+        C, = self.classes("C")
+
+        s = Session()
+        self.assert_compile(
+            s.query(C).filter_by(attr=5),
+            "SELECT c.id AS c_id, c.b_id AS c_b_id FROM c WHERE EXISTS "
+            "(SELECT 1 FROM b WHERE b.id = c.b_id AND "
+            "CAST(b.data AS INTEGER) = :param_1)",
         )