diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-04-17 16:06:04 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-04-17 16:06:04 -0400 |
commit | d349ad448c020d9c4ac976c048500d2cee51ab6d (patch) | |
tree | c35707e4e9b25c546468a418c0589a764a3495ba | |
parent | 9f468e305d0a6b42a1d384a70dcf8fb51effa693 (diff) | |
download | sqlalchemy-d349ad448c020d9c4ac976c048500d2cee51ab6d.tar.gz |
- Fixed a critical regression caused by :ticket:`3061` where the
NEVER_SET symbol could easily leak into a lazyload query, subsequent
to the flush of a pending object. This would occur typically
for a many-to-one relationship that does not use a simple
"get" strategy. The good news is that the fix improves efficiency
vs. 0.9, because we can now skip the SELECT statement entirely
when we detect NEVER_SET symbols present in the parameters; prior to
:ticket:`3061`, we couldn't discern if the None here were set or not.
fixes #3368
-rw-r--r-- | doc/build/changelog/changelog_10.rst | 17 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/base.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/strategies.py | 2 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/util.py | 2 | ||||
-rw-r--r-- | test/orm/test_lazy_relations.py | 27 |
5 files changed, 46 insertions, 4 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index d13202d71..cff3f1b5c 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -16,6 +16,23 @@ :start-line: 5 .. changelog:: + :version: 1.0.1 + + .. change:: + :tags: bug, orm + :tickets: 3368 + + Fixed a critical regression caused by :ticket:`3061` where the + NEVER_SET symbol could easily leak into a lazyload query, subsequent + to the flush of a pending object. This would occur typically + for a many-to-one relationship that does not use a simple + "get" strategy. The good news is that the fix improves efficiency + vs. 0.9, because we can now skip the SELECT statement entirely + when we detect NEVER_SET symbols present in the parameters; prior to + :ticket:`3061`, we couldn't discern if the None here were set or not. + + +.. changelog:: :version: 1.0.0 :released: April 16, 2015 diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index c259878f0..785bd09dd 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -181,6 +181,8 @@ NOT_EXTENSION = util.symbol( """) +_never_set = frozenset([NEVER_SET]) + _none_set = frozenset([None, NEVER_SET, PASSIVE_NO_RESULT]) _SET_DEFERRED_EXPIRED = util.symbol("SET_DEFERRED_EXPIRED") diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index c03e133de..5c6618686 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -585,6 +585,8 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): if pending and orm_util._none_set.intersection(params.values()): return None + elif orm_util._never_set.intersection(params.values()): + return None q = q.filter(lazy_clause).params(params) diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index b3f3bc5fa..b9098c77c 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -13,7 +13,7 @@ from . import attributes import re from .base import instance_str, state_str, state_class_str, attribute_str, \ - state_attribute_str, object_mapper, object_state, _none_set + state_attribute_str, object_mapper, object_state, _none_set, _never_set from .base import class_mapper, _class_to_mapper from .base import InspectionAttr from .path_registry import PathRegistry diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py index e99e22725..166ee90cf 100644 --- a/test/orm/test_lazy_relations.py +++ b/test/orm/test_lazy_relations.py @@ -559,7 +559,7 @@ class GetterStateTest(_fixtures.FixtureTest): run_inserts = None - def _u_ad_fixture(self, populate_user): + def _u_ad_fixture(self, populate_user, dont_use_get=False): users, Address, addresses, User = ( self.tables.users, self.classes.Address, @@ -567,9 +567,17 @@ class GetterStateTest(_fixtures.FixtureTest): self.classes.User) mapper(User, users, properties={ - 'addresses': relationship(Address, backref='user') + 'addresses': relationship(Address, back_populates='user') + }) + mapper(Address, addresses, properties={ + 'user': relationship( + User, + primaryjoin=and_( + users.c.id == addresses.c.user_id, users.c.id != 27) + if dont_use_get else None, + back_populates='addresses' + ) }) - mapper(Address, addresses) sess = create_session() a1 = Address(email_address='a1') @@ -581,6 +589,19 @@ class GetterStateTest(_fixtures.FixtureTest): sess.expire_all() return User, Address, sess, a1 + def test_no_use_get_params_missing(self): + User, Address, sess, a1 = self._u_ad_fixture(False, True) + + def go(): + eq_(a1.user, None) + + # doesn't emit SQL + self.assert_sql_count( + testing.db, + go, + 0 + ) + def test_get_empty_passive_return_never_set(self): User, Address, sess, a1 = self._u_ad_fixture(False) eq_( |