summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Hayes <pfhayes@gmail.com>2015-06-13 14:11:16 -0400
committerPatrick Hayes <pfhayes@gmail.com>2015-06-13 14:11:16 -0400
commit09485d733131b667813f44eb0b6807b698668ee7 (patch)
tree6fcb43cc5d0033fef788b52f5897a6862854bb06
parente765c55e8cc71bb3773b86b5260df6cb69aff102 (diff)
downloadsqlalchemy-09485d733131b667813f44eb0b6807b698668ee7.tar.gz
Fix primary key behaviour in bulk_updatepr/181
Suppose you have a model class with a primary key. Base = declarative_base() class User(Base): id = Column(BigInteger, primary_key=True) name = Column(String) Previously, running `bulk_update_mappings(User, {'id': 1, 'name': 'hello'})` would emit the following: ```UPDATE users SET id=1, name='hello' WHERE id=1``` This is contrary to the stated behaviour, where primary keys are omitted from the SET clause. Furthermore, this behaviour is harmful, as it can cause the db engine to lock over-aggresively (at least in Postgres). With this change, the emitted SQL is: ```UPDATE users SET name='hello' WHERE id=1```
-rw-r--r--lib/sqlalchemy/orm/persistence.py11
-rw-r--r--test/orm/test_bulk.py38
2 files changed, 39 insertions, 10 deletions
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index b429aa4c1..254d3bf09 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -472,12 +472,11 @@ def _collect_update_commands(
continue
if bulk:
- pk_params = dict(
- (propkey_to_col[propkey]._label, state_dict.get(propkey))
- for propkey in
- set(propkey_to_col).
- intersection(mapper._pk_keys_by_table[table])
- )
+ pk_params = {}
+ for propkey in set(propkey_to_col).intersection(mapper._pk_keys_by_table[table]):
+ col = propkey_to_col[propkey]
+ pk_params[col._label] = state_dict.get(propkey)
+ params.pop(col.key, None)
else:
pk_params = {}
for col in pks:
diff --git a/test/orm/test_bulk.py b/test/orm/test_bulk.py
index e27d3b73c..1e0a735c7 100644
--- a/test/orm/test_bulk.py
+++ b/test/orm/test_bulk.py
@@ -96,11 +96,41 @@ class BulkInsertUpdateTest(BulkTest, _fixtures.FixtureTest):
asserter.assert_(
CompiledSQL(
- "UPDATE users SET id=:id, name=:name WHERE "
+ "UPDATE users SET name=:name WHERE "
"users.id = :users_id",
- [{'users_id': 1, 'id': 1, 'name': 'u1new'},
- {'users_id': 2, 'id': 2, 'name': 'u2'},
- {'users_id': 3, 'id': 3, 'name': 'u3new'}]
+ [{'users_id': 1, 'name': 'u1new'},
+ {'users_id': 2, 'name': 'u2'},
+ {'users_id': 3, 'name': 'u3new'}]
+ )
+ )
+
+ def test_bulk_update(self):
+ User, = self.classes("User",)
+
+ s = Session(expire_on_commit=False)
+ objects = [
+ User(name="u1"),
+ User(name="u2"),
+ User(name="u3")
+ ]
+ s.add_all(objects)
+ s.commit()
+
+ s = Session()
+ with self.sql_execution_asserter() as asserter:
+ s.bulk_update_mappings(
+ User,
+ [{'id': 1, 'name': 'u1new'},
+ {'id': 2, 'name': 'u2'},
+ {'id': 3, 'name': 'u3new'}]
+ )
+
+ asserter.assert_(
+ CompiledSQL(
+ "UPDATE users SET name=:name WHERE users.id = :users_id",
+ [{'users_id': 1, 'name': 'u1new'},
+ {'users_id': 2, 'name': 'u2'},
+ {'users_id': 3, 'name': 'u3new'}]
)
)