after_update: only update db_version when rows are inserted into clock tbls

Author: somtochiama

core/rs/core/src/local_writes/after_update.rs

@@ -71,18 +71,20 @@ fn after_update(
     non_pks_new: &[*mut value],
     non_pks_old: &[*mut value],
 ) -> Result<ResultCode, String> {
-    let next_db_version = crate::db_version::next_db_version(db, ext_data)?;
+    let next_db_version: i64 = crate::db_version::peek_next_db_version(db, ext_data)?;
     let new_key = tbl_info
         .get_or_create_key_via_raw_values(db, pks_new)
         .map_err(|_| "failed geteting or creating lookaside key")?;
 
+    let mut changed = false;
     // Changing a primary key column to a new value is the same thing as deleting the row
     // previously identified by the primary key.
     if crate::compare_values::any_value_changed(pks_new, pks_old)? {
         let old_key = tbl_info
             .get_or_create_key_via_raw_values(db, pks_old)
             .map_err(|_| "failed geteting or creating lookaside key")?;
         let next_seq = super::bump_seq(ext_data);
+        changed = true;
         // Record the delete of the row identified by the old primary keys
         after_update__mark_old_pk_row_deleted(db, tbl_info, old_key, next_db_version, next_seq)?;
         let next_seq = super::bump_seq(ext_data);
@@ -110,6 +112,7 @@ fn after_update(
         .zip(tbl_info.non_pks.iter())
     {
         if crsql_compare_sqlite_values(*new, *old) != 0 {
+            changed = true;
             let next_seq = super::bump_seq(ext_data);
             // we had a difference in new and old values
             // we need to track crdt metadata
@@ -124,6 +127,12 @@ fn after_update(
         }
     }
 
+    // actually set the db_version
+    if changed {
+        // TODO: assert this is same as next_db_version
+        crate::db_version::next_db_version(db, ext_data)?;
+    }
+
     Ok(ResultCode::OK)
 }
 

py/correctness/tests/test_update_rows.py

@@ -65,3 +65,66 @@ def get_site_id(db):
 
     db2_foo = db2.execute("SELECT * FROM foo").fetchall()
     assert (db2_foo == db1_foo)
+
+
+def test_empty_update_doesnt_change_db_version():
+    def create_db():
+        db = connect(":memory:")
+        db.execute("CREATE TABLE foo (id INTEGER PRIMARY KEY NOT NULL, a, b)")
+        db.execute("SELECT crsql_as_crr('foo');")
+        db.commit()
+        return db
+
+    def get_site_id(db):
+        return db.execute("SELECT crsql_site_id()").fetchone()[0]
+
+    db1 = create_db()
+    db2 = create_db()
+
+    db1_site_id = get_site_id(db1)
+    db2_site_id = get_site_id(db2)
+
+    db1.execute("INSERT INTO foo (id, a, b) VALUES (1, 2, 3)")
+    db1.commit()
+
+    db1.execute("INSERT INTO foo (id, a, b) VALUES (2, 5, 6)")
+    db1.commit()
+
+    sync_left_to_right(db1, db2, 0)
+
+    db1_changes = db1.execute("SELECT * FROM crsql_changes").fetchall()
+
+    assert (db1_changes == [('foo', b'\x01\t\x01', 'a', 2, 1, 1, db1_site_id, 1, 0),
+                    ('foo', b'\x01\t\x01', 'b', 3, 1, 1, db1_site_id, 1, 1),
+                    ('foo', b'\x01\t\x02', 'a', 5, 1, 2, db1_site_id, 1, 0),
+                    ('foo', b'\x01\t\x02', 'b', 6, 1, 2, db1_site_id, 1, 1)])
+
+    db2_changes = db2.execute("SELECT * FROM crsql_changes").fetchall()
+    assert (db2_changes == db1_changes)
+
+    db1_db_version = db1.execute("SELECT crsql_db_version()").fetchone()[0]
+    assert (db1_db_version == 2)
+
+    # update row
+    db1.execute("INSERT INTO foo (id, a, b) VALUES (2, 5, 6) ON CONFLICT (id) DO UPDATE SET a = 5, b = 6")
+    db1.commit()
+
+     # update row
+    db1.execute("INSERT INTO foo (id, a, b) VALUES (1, 2, 3) ON CONFLICT (id) DO UPDATE SET a = 2, b = 3")
+    db1.commit()
+
+    db1_db_version = db1.execute("SELECT crsql_db_version()").fetchone()[0]
+    assert (db1_db_version == 2)
+
+    db1_changes = db1.execute("SELECT * FROM crsql_changes").fetchall()
+    assert (db1_changes == [('foo', b'\x01\t\x01', 'a', 2, 1, 1, db1_site_id, 1, 0),
+                ('foo', b'\x01\t\x01', 'b', 3, 1, 1, db1_site_id, 1, 1),
+                ('foo', b'\x01\t\x02', 'a', 5, 1, 2, db1_site_id, 1, 0),
+                ('foo', b'\x01\t\x02', 'b', 6, 1, 2, db1_site_id, 1, 1)])
+
+    # do an actual update
+    db1.execute("UPDATE foo SET a = 10 WHERE id = 1")
+    db1.commit()
+
+    db1_db_version = db1.execute("SELECT crsql_db_version()").fetchone()[0]
+    assert (db1_db_version == 3)