diff options
author | Mike Bayer <mike_mp@zzzcomputing.com> | 2016-02-11 22:29:18 -0500 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2016-02-11 22:31:44 -0500 |
commit | a74b028f9f5b59b7c3f2f9d6b785abf56f77609c (patch) | |
tree | e558ef437b4e9c12f6932a7b8cf5dd5229053baa | |
parent | 3215a2dac0e54624a17a96a2caae8ff74ab1e4ba (diff) | |
download | sqlalchemy-a74b028f9f5b59b7c3f2f9d6b785abf56f77609c.tar.gz |
- Fixed bug in :meth:`.Session.merge` where an object with a composite
primary key that has values for some but not all of the PK fields
would emit a SELECT statement leaking the internal NEVER_SET symbol
into the query, rather than detecting that this object does not have
a searchable primary key and no SELECT should be emitted.
fixes #3647
(cherry picked from commit 366f97b5617af0d15cfaf594ec5ef0408c70e873)
-rw-r--r-- | doc/build/changelog/changelog_10.rst | 10 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/session.py | 8 | ||||
-rw-r--r-- | test/orm/test_merge.py | 51 |
3 files changed, 66 insertions, 3 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index d607110aa..ac250a7aa 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -20,6 +20,16 @@ :released: .. change:: + :tags: bug, orm + :tickets: 3647 + + Fixed bug in :meth:`.Session.merge` where an object with a composite + primary key that has values for some but not all of the PK fields + would emit a SELECT statement leaking the internal NEVER_SET symbol + into the query, rather than detecting that this object does not have + a searchable primary key and no SELECT should be emitted. + + .. change:: :tags: bug, postgresql :tickets: 3644 diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 2c1bdedee..000441fb9 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -1726,6 +1726,9 @@ class Session(_SessionClassMethods): "all changes on mapped instances before merging with " "load=False.") key = mapper._identity_key_from_state(state) + key_is_persistent = attributes.NEVER_SET not in key[1] + else: + key_is_persistent = True if key in self.identity_map: merged = self.identity_map[key] @@ -1742,9 +1745,10 @@ class Session(_SessionClassMethods): self._update_impl(merged_state) new_instance = True - elif not _none_set.intersection(key[1]) or \ + elif key_is_persistent and ( + not _none_set.intersection(key[1]) or (mapper.allow_partial_pks and - not _none_set.issuperset(key[1])): + not _none_set.issuperset(key[1]))): merged = self.query(mapper.class_).get(key[1]) else: merged = None diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index a52274896..14b1b0513 100644 --- a/test/orm/test_merge.py +++ b/test/orm/test_merge.py @@ -6,7 +6,7 @@ from sqlalchemy import testing from sqlalchemy.util import OrderedSet from sqlalchemy.orm import mapper, relationship, create_session, \ PropComparator, synonym, comparable_property, sessionmaker, \ - attributes, Session, backref, configure_mappers + attributes, Session, backref, configure_mappers, foreign from sqlalchemy.orm.collections import attribute_mapped_collection from sqlalchemy.orm.interfaces import MapperOption from sqlalchemy.testing import eq_, ne_ @@ -451,6 +451,55 @@ class MergeTest(_fixtures.FixtureTest): eq_(u2.addresses[1].email_address, 'afafds') eq_(load.called, 21) + def test_dont_send_neverset_to_get(self): + # test issue #3647 + CompositePk, composite_pk_table = ( + self.classes.CompositePk, self.tables.composite_pk_table + ) + mapper(CompositePk, composite_pk_table) + cp1 = CompositePk(j=1, k=1) + + sess = Session() + + rec = [] + + def go(): + rec.append(sess.merge(cp1)) + self.assert_sql_count(testing.db, go, 0) + rec[0].i = 5 + sess.commit() + eq_(rec[0].i, 5) + + def test_dont_send_neverset_to_get_w_relationship(self): + # test issue #3647 + CompositePk, composite_pk_table = ( + self.classes.CompositePk, self.tables.composite_pk_table + ) + User, users = ( + self.classes.User, self.tables.users + ) + mapper(User, users, properties={ + 'elements': relationship( + CompositePk, + primaryjoin=users.c.id == foreign(composite_pk_table.c.i)) + }) + mapper(CompositePk, composite_pk_table) + + u1 = User(id=5, name='some user') + cp1 = CompositePk(j=1, k=1) + u1.elements.append(cp1) + sess = Session() + + rec = [] + + def go(): + rec.append(sess.merge(u1)) + self.assert_sql_count(testing.db, go, 1) + u2 = rec[0] + sess.commit() + eq_(u2.elements[0].i, 5) + eq_(u2.id, 5) + def test_no_relationship_cascade(self): """test that merge doesn't interfere with a relationship() target that specifically doesn't include 'merge' cascade. |