From: Mike Bayer Date: Sun, 19 May 2019 16:38:14 +0000 (-0400) Subject: Add QueryableAttribute._impl_uses_objects accessor for AssociationProxy X-Git-Tag: rel_1_4_0b1~868 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c785a528ea200a8905d1b5d7ab4088d501606d2b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..e9aa4bef9d --- /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 c725372c5c..9a8294f3c7 100644 --- 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 f37928cc1e..674c57feac 100644 --- 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 683a84fcdf..d515ccf656 100644 --- 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)", )