make site_id required

1. We compact out side ids so there isn't much advantage to making it null for local writes anymore
2. This cleans up the interface so users don't have to do weird coalescing
3. This allows us to make #386 a reality by switching tie breaks to site_ids

README.md

@@ -68,8 +68,8 @@ The full documentation site is available [here](https://vlcn.io/docs).
 - A function extension (`crsql_as_crr`) to upgrade existing tables to "crrs" or "conflict free replicated relations"
   - `SELECT crsql_as_crr('table_name')`
 - A virtual table (`crsql_changes`) to ask the database for changesets or to apply changesets from another database
-  - `SELECT "table", "pk", "cid", "val", "col_version", "db_version", COALESCE("site_id", crsql_site_id()), cl, seq FROM crsql_changes WHERE db_version > x AND site_id IS NULL` -- to get local changes
-  - `SELECT "table", "pk", "cid", "val", "col_version", "db_version", COALESCE("site_id", crsql_site_id()), cl, seq FROM crsql_changes WHERE db_version > x AND site_id IS NOT some_site` -- to get all changes excluding those synced from some site
+  - `SELECT "table", "pk", "cid", "val", "col_version", "db_version", "site_id", cl, seq FROM crsql_changes WHERE db_version > x AND site_id IS NULL` -- to get local changes
+  - `SELECT "table", "pk", "cid", "val", "col_version", "db_version", "site_id", cl, seq FROM crsql_changes WHERE db_version > x AND site_id IS NOT some_site` -- to get all changes excluding those synced from some site
   - `INSERT INTO crsql_changes VALUES ([patches received from select on another peer])`
 - And `crsql_begin_alter('table_name')` & `crsql_alter_commit('table_name')` primitives to allow altering table definitions that have been upgraded to `crr`s.
   - Until we move forward with extending the syntax of SQLite to be CRR aware, altering CRRs looks like:
@@ -104,10 +104,10 @@ insert into foo (a,b) values (1,2);
 insert into baz (a,b,c,d) values ('a', 'woo', 'doo', 'daa');
 
 -- ask for a record of what has changed
-select "table", "pk", "cid", "val", "col_version", "db_version", COALESCE("site_id", crsql_site_id()), "cl", "seq" from crsql_changes;
+select "table", "pk", "cid", "val", "col_version", "db_version", "site_id", "cl", "seq" from crsql_changes;
 
 ┌───────┬─────────────┬─────┬───────┬─────────────┬────────────┬──────────────────────────────────────┬────┬─────┐
-│ table │     pk      │ cid │  val  │ col_version │ db_version │ COALESCE("site_id", crsql_site_id()) │ cl │ seq │
+│ table │     pk      │ cid │  val  │ col_version │ db_version │ "site_id" │ cl │ seq │
 ├───────┼─────────────┼─────┼───────┼─────────────┼────────────┼──────────────────────────────────────┼────┼─────┤
 │ 'foo' │ x'010901'   │ 'b' │ 2     │ 1           │ 1          │ x'049c48eadf4440d7944ed9ec88b13ea5'  │ 1  │ 0   │
 │ 'baz' │ x'010b0161' │ 'b' │ 'woo' │ 1           │ 2          │ x'049c48eadf4440d7944ed9ec88b13ea5'  │ 1  │ 0   │

core/rs/core/src/bootstrap.rs

@@ -209,7 +209,7 @@ pub fn create_clock_table(
       site_id INTEGER,
       seq INTEGER NOT NULL,
       PRIMARY KEY (key, col_name)
-    ) WITHOUT ROWID",
+    ) WITHOUT ROWID, STRICT",
         table_name = crate::util::escape_ident(table_name),
     ))?;
 

core/rs/core/src/tableinfo.rs

@@ -421,12 +421,12 @@ impl TableInfo {
             2,
             ?,
             ?,
-            NULL WHERE true
+            0 WHERE true
           ON CONFLICT DO UPDATE SET
             col_version = 1 + col_version,
             db_version = ?,
             seq = ?,
-            site_id = crsql_site_id()",
+            site_id = 0",
                 table_name = crate::util::escape_ident(&self.tbl_name),
                 sentinel = crate::c::DELETE_SENTINEL,
             );
@@ -471,12 +471,12 @@ impl TableInfo {
                 1,
                 ?,
                 ?,
-                NULL WHERE true
+                0 WHERE true
                 ON CONFLICT DO UPDATE SET
                   col_version = CASE col_version % 2 WHEN 0 THEN col_version + 1 ELSE col_version + 2 END,
                   db_version = ?,
                   seq = ?,
-                  site_id = NULL",
+                  site_id = 0",
               table_name = crate::util::escape_ident(&self.tbl_name),
               sentinel = crate::c::INSERT_SENTINEL,
             );
@@ -505,12 +505,12 @@ impl TableInfo {
               1,
               ?,
               ?,
-              NULL WHERE true
+              0 WHERE true
             ON CONFLICT DO UPDATE SET
               col_version = col_version + 1,
               db_version = ?,
               seq = ?,
-              site_id = NULL;",
+              site_id = 0;",
                 table_name = crate::util::escape_ident(&self.tbl_name),
             );
             let ret = db.prepare_v3(&sql, sqlite::PREPARE_PERSISTENT)?;
@@ -533,7 +533,7 @@ impl TableInfo {
                 col_version = CASE col_version % 2 WHEN 0 THEN col_version + 1 ELSE col_version + 2 END,
                 db_version = ?,
                 seq = ?,
-                site_id = NULL
+                site_id = 0
               WHERE key = ? AND col_name = ?",
               table_name = crate::util::escape_ident(&self.tbl_name),
             );

core/src/changes-vtab.test.c

@@ -83,16 +83,16 @@ static void testFilters() {
 
   printf("is null\n");
   assertCount(db, "SELECT count(*) FROM crsql_changes WHERE site_id IS NULL",
-              3);
+              0);
 
   printf("is not null\n");
   assertCount(
-      db, "SELECT count(*) FROM crsql_changes WHERE site_id IS NOT NULL", 0);
+      db, "SELECT count(*) FROM crsql_changes WHERE site_id IS NOT NULL", 3);
 
   printf("equals\n");
   assertCount(
       db, "SELECT count(*) FROM crsql_changes WHERE site_id = crsql_site_id()",
-      0);
+      3);
 
   // 0 rows is actually correct ANSI sql behavior. NULLs are never equal, or not
   // equal, to anything in ANSI SQL. So users must use `IS NOT` to check rather
@@ -105,10 +105,10 @@ static void testFilters() {
       0);
 
   printf("is not\n");
-  // All rows are currently null for site_id
+  // All rows are currently equal to site_id since all rows are currently local
+  // writes
   assertCount(
-      db,
-      "SELECT count(*) FROM crsql_changes WHERE site_id IS NOT crsql_site_id()",
+      db, "SELECT count(*) FROM crsql_changes WHERE site_id IS crsql_site_id()",
       3);
 
   // compare on db_version _and_ site_id
@@ -123,7 +123,7 @@ static void testFilters() {
   printf("OR condition\n");
   assertCount(db,
               "SELECT count(*) FROM crsql_changes WHERE db_version > 2 OR "
-              "site_id IS NULL",
+              "site_id IS crsql_site_id()",
               3);
 
   // compare on pks, table name, other not perfectly supported columns

core/src/crsqlite.test.c

@@ -229,15 +229,16 @@ static void teste2e() {
   assert(rc == SQLITE_ROW);
 
   const char *tmpSiteid = (const char *)sqlite3_column_text(pStmt3, 0);
-  // printf("db1sid: %s\n", db1siteid);
-  // printf("db2sid: %s\n", db2siteid);
-  // printf("db3sid: %s\n", db3siteid);
-  // printf("tempsid: %s\n", tmpSiteid);
-  assert(strcmp(tmpSiteid, "NULL") == 0);
+  printf("db1sid: %s\n", db1siteid);
+  printf("db2sid: %s\n", db2siteid);
+  printf("db3sid: %s\n", db3siteid);
+  printf("tempsid: %s\n", tmpSiteid);
+  // printf("tmp: %s, db3: %s", tmpSiteid, db3siteid);
+  assert(strcmp(tmpSiteid, db3siteid) == 0);
 
   rc = sqlite3_step(pStmt3);
   assert(rc == SQLITE_ROW);
-  assert(strcmp((const char *)sqlite3_column_text(pStmt3, 0), "NULL") == 0);
+  assert(strcmp((const char *)sqlite3_column_text(pStmt3, 0), db3siteid) == 0);
   sqlite3_finalize(pStmt3);
 
   rc = sqlite3_prepare_v2(db2, "SELECT * FROM foo ORDER BY a ASC", -1, &pStmt2,

py/correctness/tests/test_cl_merging.py

@@ -56,7 +56,7 @@ def sync_left_to_right_exact_version(l, r, db_version):
 
 def sync_left_to_right_include_siteid(l, r, since):
     changes = l.execute(
-        "SELECT [table], pk, cid, val, col_version, db_version, coalesce(site_id, crsql_site_id()), cl, seq FROM crsql_changes WHERE db_version > ?", (since,))
+        "SELECT [table], pk, cid, val, col_version, db_version, site_id, cl, seq FROM crsql_changes WHERE db_version > ?", (since,))
     for change in changes:
         r.execute(
             "INSERT INTO crsql_changes VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", change)
@@ -69,7 +69,7 @@ def sync_left_to_right_include_siteid(l, r, since):
 def sync_left_to_right_normal_delta_state(l, r, since):
     r_siteid = r.execute("SELECT crsql_site_id()").fetchone()[0]
     changes = l.execute(
-        "SELECT [table], pk, cid, val, col_version, db_version, coalesce(site_id, crsql_site_id()), cl, seq FROM crsql_changes WHERE db_version > ? AND site_id IS NOT ?",
+        "SELECT [table], pk, cid, val, col_version, db_version, site_id, cl, seq FROM crsql_changes WHERE db_version > ? AND site_id IS NOT ?",
         (since, r_siteid))
     largest_version = 0
     for change in changes:
@@ -83,7 +83,7 @@ def sync_left_to_right_normal_delta_state(l, r, since):
 def sync_left_to_right_single_vrsn(l, r, vrsn):
     r_siteid = r.execute("SELECT crsql_site_id()").fetchone()[0]
     changes = l.execute(
-        "SELECT [table], pk, cid, val, col_version, db_version, coalesce(site_id, crsql_site_id()), cl, seq FROM crsql_changes WHERE db_version = ? AND site_id IS NOT ?",
+        "SELECT [table], pk, cid, val, col_version, db_version, site_id, cl, seq FROM crsql_changes WHERE db_version = ? AND site_id IS NOT ?",
         (vrsn, r_siteid))
     for change in changes:
         r.execute(
@@ -276,6 +276,7 @@ def test_pr_299_scenario():
     # c2 should have accepted all the changes given the higher causal length
     # a = 1, b = 1, cl = 3
     # note: why is site_id missing??? Ah, it is missing since we don't coalesce to get it. This is expected.
+    # TODO: should no longer be missing site_id!
     assert (changes == [('foo', b'\x01\t\x01', '-1', None, 3, 3, None, 3, 0),
                         ('foo', b'\x01\t\x01', 'b', 1, 1, 3, None, 3, 1)])
     # c2 and c1 should match in terms of data