From b73fc8f874da94c9c5b2d94feb6b1b45b7f4f02b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 16 Apr 2021 08:56:17 -0400 Subject: [PATCH] synonym fixes and enhancements Fixed regression where an attribute that is mapped to a :func:`_orm.synonym` could not be used in column loader options such as :func:`_orm.load_only`. Established support for :func:`_orm.synonym` in conjunction with hybrid property, associationproxy, including that synonyms can be established linking to these constructs which work fully. This is a behavior that was semi-explicitly disallowed previously, however since it did not fail in every scenario, explicit support for assoc proxy and hybrids has been added. Fixes: #6272 Fixes: #6267 Change-Id: Ie75bb3b535feeb6ccf3f6a391f21b69f241e625e --- .../changelog/unreleased_14/6272_etc.rst | 19 +++++ lib/sqlalchemy/orm/attributes.py | 2 + lib/sqlalchemy/orm/descriptor_props.py | 25 ++++-- test/ext/test_associationproxy.py | 51 +++++++++++- test/ext/test_hybrid.py | 77 +++++++++++++++++++ test/orm/test_deferred.py | 24 ++++++ test/orm/test_mapper.py | 16 ++-- 7 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6272_etc.rst diff --git a/doc/build/changelog/unreleased_14/6272_etc.rst b/doc/build/changelog/unreleased_14/6272_etc.rst new file mode 100644 index 0000000000..e840629e43 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6272_etc.rst @@ -0,0 +1,19 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 6272 + + Fixed regression where an attribute that is mapped to a + :func:`_orm.synonym` could not be used in column loader options such as + :func:`_orm.load_only`. + +.. change:: + :tags: usecase, orm + :tickets: 6267 + + Established support for :func:`_orm.synoynm` in conjunction with + hybrid property, assocaitionproxy is set up completely, including that + synonyms can be established linking to these constructs which work + fully. This is a behavior that was semi-explicitly disallowed previously, + however since it did not fail in every scenario, explicit support + for assoc proxy and hybrids has been added. + diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index d1ed17f1a8..9e326db64a 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -487,6 +487,8 @@ def create_proxied_attribute(descriptor): """ + _extra_criteria = () + def __init__( self, class_, diff --git a/lib/sqlalchemy/orm/descriptor_props.py b/lib/sqlalchemy/orm/descriptor_props.py index c306725669..f8c42ee606 100644 --- a/lib/sqlalchemy/orm/descriptor_props.py +++ b/lib/sqlalchemy/orm/descriptor_props.py @@ -22,6 +22,7 @@ from .. import schema from .. import sql from .. import util from ..sql import expression +from ..sql import operators class DescriptorProperty(MapperProperty): @@ -665,15 +666,22 @@ class SynonymProperty(DescriptorProperty): def uses_objects(self): return getattr(self.parent.class_, self.name).impl.uses_objects - # TODO: when initialized, check _proxied_property, + # TODO: when initialized, check _proxied_object, # emit a warning if its not a column-based property @util.memoized_property - def _proxied_property(self): + def _proxied_object(self): attr = getattr(self.parent.class_, self.name) if not hasattr(attr, "property") or not isinstance( attr.property, MapperProperty ): + # attribute is a non-MapperProprerty proxy such as + # hybrid or association proxy + if isinstance(attr, attributes.QueryableAttribute): + return attr.comparator + elif isinstance(attr, operators.ColumnOperators): + return attr + raise sa_exc.InvalidRequestError( """synonym() attribute "%s.%s" only supports """ """ORM mapped attributes, got %r""" @@ -682,13 +690,16 @@ class SynonymProperty(DescriptorProperty): return attr.property def _comparator_factory(self, mapper): - prop = self._proxied_property + prop = self._proxied_object - if self.comparator_factory: - comp = self.comparator_factory(prop, mapper) + if isinstance(prop, MapperProperty): + if self.comparator_factory: + comp = self.comparator_factory(prop, mapper) + else: + comp = prop.comparator_factory(prop, mapper) + return comp else: - comp = prop.comparator_factory(prop, mapper) - return comp + return prop def get_history(self, *arg, **kw): attr = getattr(self.parent.class_, self.name) diff --git a/test/ext/test_associationproxy.py b/test/ext/test_associationproxy.py index 31ae050c11..db837e6b66 100644 --- a/test/ext/test_associationproxy.py +++ b/test/ext/test_associationproxy.py @@ -30,9 +30,10 @@ from sqlalchemy.testing import assert_raises from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ -from sqlalchemy.testing.assertions import expect_warnings +from sqlalchemy.testing import is_false from sqlalchemy.testing.fixtures import fixture_session from sqlalchemy.testing.mock import call from sqlalchemy.testing.mock import Mock @@ -3248,6 +3249,54 @@ class ProxyOfSynonymTest(AssertsCompiledSQL, fixtures.DeclarativeMappedTest): ) +class SynonymOfProxyTest(AssertsCompiledSQL, fixtures.DeclarativeMappedTest): + __dialect__ = "default" + + run_create_tables = None + + @classmethod + def setup_classes(cls): + from sqlalchemy.orm import synonym + + Base = cls.DeclarativeBasic + + class A(Base): + __tablename__ = "a" + + id = Column(Integer, primary_key=True) + data = Column(String) + bs = relationship("B", backref="a") + + b_data = association_proxy("bs", "data") + + b_data_syn = synonym("b_data") + + class B(Base): + __tablename__ = "b" + id = Column(Integer, primary_key=True) + a_id = Column(ForeignKey("a.id")) + data = Column(String) + + def test_hasattr(self): + A, B = self.classes("A", "B") + is_false(hasattr(A.b_data_syn, "nonexistent")) + + def test_o2m_instance_getter(self): + A, B = self.classes("A", "B") + + a1 = A(bs=[B(data="bdata1"), B(data="bdata2")]) + eq_(a1.b_data_syn, ["bdata1", "bdata2"]) + + def test_o2m_expr(self): + A, B = self.classes("A", "B") + + self.assert_compile( + A.b_data_syn == "foo", + "EXISTS (SELECT 1 FROM a, b WHERE a.id = b.a_id " + "AND b.data = :data_1)", + ) + + class ProxyHybridTest(fixtures.DeclarativeMappedTest, AssertsCompiledSQL): __dialect__ = "default" diff --git a/test/ext/test_hybrid.py b/test/ext/test_hybrid.py index 8be8023475..d44e284a34 100644 --- a/test/ext/test_hybrid.py +++ b/test/ext/test_hybrid.py @@ -4,6 +4,7 @@ from sqlalchemy import ForeignKey from sqlalchemy import func from sqlalchemy import inspect from sqlalchemy import Integer +from sqlalchemy import literal_column from sqlalchemy import Numeric from sqlalchemy import select from sqlalchemy import String @@ -12,12 +13,14 @@ from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import aliased from sqlalchemy.orm import relationship from sqlalchemy.orm import Session +from sqlalchemy.orm import synonym from sqlalchemy.sql import update from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ +from sqlalchemy.testing import is_false from sqlalchemy.testing.fixtures import fixture_session from sqlalchemy.testing.schema import Column @@ -492,6 +495,80 @@ class PropertyMirrorTest(fixtures.TestBase, AssertsCompiledSQL): is_(insp.all_orm_descriptors["value"].info, A.value.info) +class SynonymOfPropertyTest(fixtures.TestBase, AssertsCompiledSQL): + __dialect__ = "default" + + def _fixture(self): + Base = declarative_base() + + class A(Base): + __tablename__ = "a" + id = Column(Integer, primary_key=True) + _value = Column("value", String) + + @hybrid.hybrid_property + def value(self): + return self._value + + value_syn = synonym("value") + + @hybrid.hybrid_property + def string_value(self): + return "foo" + + string_value_syn = synonym("string_value") + + @hybrid.hybrid_property + def string_expr_value(self): + return "foo" + + @string_expr_value.expression + def string_expr_value(cls): + return literal_column("'foo'") + + string_expr_value_syn = synonym("string_expr_value") + + return A + + def test_hasattr(self): + A = self._fixture() + + is_false(hasattr(A.value_syn, "nonexistent")) + + is_false(hasattr(A.string_value_syn, "nonexistent")) + + is_false(hasattr(A.string_expr_value_syn, "nonexistent")) + + def test_instance_access(self): + A = self._fixture() + + a1 = A(_value="hi") + + eq_(a1.value_syn, "hi") + + eq_(a1.string_value_syn, "foo") + + eq_(a1.string_expr_value_syn, "foo") + + def test_expression_property(self): + A = self._fixture() + + self.assert_compile( + select(A.id, A.value_syn).where(A.value_syn == "value"), + "SELECT a.id, a.value FROM a WHERE a.value = :value_1", + ) + + def test_expression_expr(self): + A = self._fixture() + + self.assert_compile( + select(A.id, A.string_expr_value_syn).where( + A.string_expr_value_syn == "value" + ), + "SELECT a.id, 'foo' FROM a WHERE 'foo' = :'foo'_1", + ) + + class MethodExpressionTest(fixtures.TestBase, AssertsCompiledSQL): __dialect__ = "default" diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py index decb456c61..7b8166c2e8 100644 --- a/test/orm/test_deferred.py +++ b/test/orm/test_deferred.py @@ -1114,6 +1114,30 @@ class DeferredOptionsTest(AssertsCompiledSQL, _fixtures.FixtureTest): "orders.isopen AS orders_isopen FROM orders", ) + @testing.combinations(("string",), ("attr",)) + def test_load_only_synonym(self, type_): + orders, Order = self.tables.orders, self.classes.Order + + mapper( + Order, + orders, + properties={"desc": synonym("description")}, + ) + + if type_ == "attr": + opt = load_only(Order.isopen, Order.desc) + else: + opt = load_only("isopen", "desc") + + sess = fixture_session() + q = sess.query(Order).options(opt) + self.assert_compile( + q, + "SELECT orders.id AS orders_id, orders.description " + "AS orders_description, orders.isopen AS orders_isopen " + "FROM orders", + ) + def test_load_only_propagate_unbound(self): self._test_load_only_propagate(False) diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 3c8f83f919..b9164111b7 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -1662,10 +1662,10 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): assert hasattr(User.x, "comparator") def test_synonym_of_non_property_raises(self): - from sqlalchemy.ext.associationproxy import association_proxy - class User(object): - pass + @property + def x(self): + return "hi" users, Address, addresses = ( self.tables.users, @@ -1679,17 +1679,23 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL): properties={"y": synonym("x"), "addresses": relationship(Address)}, ) self.mapper(Address, addresses) - User.x = association_proxy("addresses", "email_address") assert_raises_message( sa.exc.InvalidRequestError, r'synonym\(\) attribute "User.x" only supports ORM mapped ' - "attributes, got .*AssociationProxy", + "attributes, got .*property", getattr, User.y, "property", ) + assert_raises_message( + sa.exc.InvalidRequestError, + r'synonym\(\) attribute "User.x" only supports ORM mapped ' + "attributes, got .*property", + lambda: User.y == 10, + ) + def test_synonym_column_location(self): users, User = self.tables.users, self.classes.User -- 2.47.3