- 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.

diff --git a/CHANGES b/CHANGES
index faf2ee2c40dcc150d048f15abc30f996ef5b72c3..db59e629e76dec6967fd81fe4bc760d6d5510247 100644 (file)
--- 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 941c303d777f2cda90fe28eea27c7b4b0905e7b1..2bdf2b84764e13634062fbc74f24abbd155b4681 100644 (file)
--- 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 768ffeebadb26550d6ef8c6e71b0459d9ea3bd4a..b4f1bba8055c8d5ac16aab6141b5d8423c3e3540 100644 (file)
--- 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