From b69cbec1355a0eef435f651aeabe3eddce2c3ce9 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 5 Feb 2010 19:16:10 +0000 Subject: [PATCH] - Slight improvement to the fix for [ticket:1362] to not issue needless updates of the primary key column during a so-called "row switch" operation, i.e. add + delete of two objects with the same PK. --- CHANGES | 5 +++++ lib/sqlalchemy/orm/mapper.py | 25 ++++++++++++------------ test/orm/test_naturalpks.py | 37 +++++++++++++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index faf2ee2c40..db59e629e7 100644 --- a/CHANGES +++ b/CHANGES @@ -13,6 +13,11 @@ CHANGES internally. The formerly "pending" objects are now expunged first. [ticket:1674] + - Slight improvement to the fix for [ticket:1362] to not issue + needless updates of the primary key column during a so-called + "row switch" operation, i.e. add + delete of two objects + with the same PK. + - sql - Added math negation operator support, -x. diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 941c303d77..2bdf2b8476 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1408,19 +1408,18 @@ class Mapper(object): params[col.key] = prop.get_col_value(col, history.added[0]) if col in pks: - # if passive_updates and sync detected this was a - # pk->pk sync, use the new value to locate the row, - # since the DB would already have set this - if ("pk_cascaded", state, col) in \ - uowtransaction.attributes: - params[col._label] = \ - prop.get_col_value(col, history.added[0]) - hasdata = True - - elif history.deleted: - # PK is changing - use the old value to locate the row - params[col._label] = \ - prop.get_col_value(col, history.deleted[0]) + if history.deleted: + # if passive_updates and sync detected this was a + # pk->pk sync, use the new value to locate the row, + # since the DB would already have set this + if ("pk_cascaded", state, col) in \ + uowtransaction.attributes: + params[col._label] = \ + prop.get_col_value(col, history.added[0]) + else: + # use the old value to locate the row + params[col._label] = \ + prop.get_col_value(col, history.deleted[0]) hasdata = True else: # row switch logic can reach us here diff --git a/test/orm/test_naturalpks.py b/test/orm/test_naturalpks.py index 768ffeebad..b4f1bba805 100644 --- a/test/orm/test_naturalpks.py +++ b/test/orm/test_naturalpks.py @@ -557,7 +557,7 @@ class NonPKCascadeTest(_base.MappedTest): eq_(User(username='fred', fullname='jack'), u1) -class CascadeToFKPKTest(_base.MappedTest): +class CascadeToFKPKTest(_base.MappedTest, testing.AssertsCompiledSQL): """A primary key mutation cascades onto a foreign key that is itself a primary key.""" @classmethod @@ -577,6 +577,7 @@ class CascadeToFKPKTest(_base.MappedTest): primary_key=True ), Column('email', String(50), primary_key=True), + Column('etc', String(50)), test_needs_fk=True ) @@ -626,6 +627,40 @@ class CascadeToFKPKTest(_base.MappedTest): a1.username = 'jack' sess.flush() + @testing.resolve_artifact_names + def test_rowswitch_doesntfire(self): + mapper(User, users) + mapper(Address, addresses, properties={ + 'user':relation(User, passive_updates=True) + }) + + sess = create_session() + u1 = User(username='ed') + a1 = Address(user=u1, email='ed@host1') + + sess.add(u1) + sess.add(a1) + sess.flush() + + sess.delete(u1) + sess.delete(a1) + + u2 = User(username='ed') + a2 = Address(user=u2, email='ed@host1', etc='foo') + sess.add(u2) + sess.add(a2) + + from sqlalchemy.test.assertsql import CompiledSQL + + # test that the primary key columns of addresses are not + # being updated as well, since this is a row switch. + self.assert_sql_execution(testing.db, + sess.flush, + CompiledSQL("UPDATE addresses SET etc=:etc WHERE " + "addresses.username = :addresses_username AND" + " addresses.email = :addresses_email", + {'etc': 'foo', 'addresses_username':'ed', 'addresses_email':'ed@host1'} ), + ) @testing.resolve_artifact_names -- 2.47.3