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/tableinfo.rs

@@ -421,7 +421,7 @@ impl TableInfo {
             2,
             ?,
             ?,
-            NULL WHERE true
+            crsql_site_id() WHERE true
           ON CONFLICT DO UPDATE SET
             col_version = 1 + col_version,
             db_version = ?,

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