diff options
| author | mike bayer <mike_mp@zzzcomputing.com> | 2017-09-05 09:46:05 -0400 |
|---|---|---|
| committer | Gerrit Code Review <gerrit@awstats.zzzcomputing.com> | 2017-09-05 09:46:05 -0400 |
| commit | ba9c238d7e53709d64516a7dcda8b312d15f08e5 (patch) | |
| tree | 270b7293fa160a05a29df362d721c3ac8251ced9 | |
| parent | 04958bb7d7985ff8f09ac30a5b592d013bab8476 (diff) | |
| parent | 68953902dda9209ee72cde44d4802d730e4f75e3 (diff) | |
| download | sqlalchemy-ba9c238d7e53709d64516a7dcda8b312d15f08e5.tar.gz | |
Merge "Guard against KeyError in session.merge after check for identity" into rel_1_1
| -rw-r--r-- | doc/build/changelog/unreleased_11/4069.rst | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 42 |
2 files changed, 33 insertions, 18 deletions
diff --git a/doc/build/changelog/unreleased_11/4069.rst b/doc/build/changelog/unreleased_11/4069.rst new file mode 100644 index 000000000..dadae423c --- /dev/null +++ b/doc/build/changelog/unreleased_11/4069.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 4069 + :versions: 1.2.0b3 + + Fixed bug in :meth:`.Session.merge` following along similar lines as that + of :ticket:`4030`, where an internal check for a target object in + the identity map could lead to an error if it were to be garbage collected + immediately before the merge routine actually retrieves the object.
\ No newline at end of file diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index ebf1f61f1..2c040561b 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1875,28 +1875,34 @@ class Session(_SessionClassMethods): key_is_persistent = True if key in self.identity_map: - merged = self.identity_map[key] - elif key_is_persistent and key in _resolve_conflict_map: - merged = _resolve_conflict_map[key] - - elif not load: - if state.modified: - raise sa_exc.InvalidRequestError( - "merge() with load=False option does not support " - "objects marked as 'dirty'. flush() all changes on " - "mapped instances before merging with load=False.") - merged = mapper.class_manager.new_instance() - merged_state = attributes.instance_state(merged) - merged_state.key = key - self._update_impl(merged_state) - new_instance = True - - elif key_is_persistent: - merged = self.query(mapper.class_).get(key[1]) + try: + merged = self.identity_map[key] + except KeyError: + # object was GC'ed right as we checked for it + merged = None else: merged = None if merged is None: + if key_is_persistent and key in _resolve_conflict_map: + merged = _resolve_conflict_map[key] + + elif not load: + if state.modified: + raise sa_exc.InvalidRequestError( + "merge() with load=False option does not support " + "objects marked as 'dirty'. flush() all changes on " + "mapped instances before merging with load=False.") + merged = mapper.class_manager.new_instance() + merged_state = attributes.instance_state(merged) + merged_state.key = key + self._update_impl(merged_state) + new_instance = True + + elif key_is_persistent: + merged = self.query(mapper.class_).get(key[1]) + + if merged is None: merged = mapper.class_manager.new_instance() merged_state = attributes.instance_state(merged) merged_dict = attributes.instance_dict(merged) |