diff --git a/libsql-sqlite3/ext/crr/Makefile b/libsql-sqlite3/ext/crr/Makefile index bc288b870b..1935dca358 100644 --- a/libsql-sqlite3/ext/crr/Makefile +++ b/libsql-sqlite3/ext/crr/Makefile @@ -4,8 +4,10 @@ else CC:=$(CI_GCC) endif -LOADABLE_CFLAGS=-std=c99 -fPIC -shared -Wall -STATIC_CFLAGS=-std=c99 -fPIC -c -Wall +SHARED_CFLAGS=-DLIBSQL=1 +LOADABLE_CFLAGS=-std=c99 -fPIC -shared -Wall $(SHARED_CFLAGS) +STATIC_CFLAGS=-std=c99 -fPIC -c -Wall $(SHARED_CFLAGS) +libsql_feature=,libsql ifeq ($(shell uname -s),Darwin) CONFIG_DARWIN=y @@ -169,19 +171,19 @@ $(sqlite3.c): cd $(sqlite_src) && make sqlite3.c $(rs_lib_dbg_static_cpy): FORCE $(dbg_prefix) - cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --features static,omit_load_extension $(rs_build_flags) + cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --features static,omit_load_extension$(libsql_feature) $(rs_build_flags) cp $(rs_lib_dbg_static) $(rs_lib_dbg_static_cpy) $(rs_lib_static_cpy): FORCE $(prefix) - cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --release --features static,omit_load_extension $(rs_build_flags) + cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --release --features static,omit_load_extension$(libsql_feature) $(rs_build_flags) cp $(rs_lib_static) $(rs_lib_static_cpy) $(rs_lib_loadable_cpy): FORCE $(prefix) - cd ./rs/$(bundle) && cargo $(rs_ndk) build $(RS_TARGET) --release --features loadable_extension $(rs_build_flags) + cd ./rs/$(bundle) && cargo $(rs_ndk) build $(RS_TARGET) --release --features loadable_extension$(libsql_feature) $(rs_build_flags) cp $(rs_lib_loadable) $(rs_lib_loadable_cpy) $(rs_lib_dbg_loadable_cpy): FORCE $(dbg_prefix) - cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --features loadable_extension $(rs_build_flags) + cd ./rs/$(bundle) && cargo rustc $(RS_TARGET) --features loadable_extension$(libsql_feature) $(rs_build_flags) cp $(rs_lib_dbg_loadable) $(rs_lib_dbg_loadable_cpy) # Build the loadable extension. diff --git a/libsql-sqlite3/ext/crr/rs/bundle/Cargo.toml b/libsql-sqlite3/ext/crr/rs/bundle/Cargo.toml index 2edc1174db..a5bcc151ab 100644 --- a/libsql-sqlite3/ext/crr/rs/bundle/Cargo.toml +++ b/libsql-sqlite3/ext/crr/rs/bundle/Cargo.toml @@ -25,6 +25,7 @@ panic = "abort" [features] test = ["crsql_core/test"] +libsql = ["crsql_core/libsql"] loadable_extension = [ "sqlite_nostd/loadable_extension", "crsql_fractindex_core/loadable_extension", diff --git a/libsql-sqlite3/ext/crr/rs/bundle_static/Cargo.toml b/libsql-sqlite3/ext/crr/rs/bundle_static/Cargo.toml index ad3762d360..2e07f5136a 100644 --- a/libsql-sqlite3/ext/crr/rs/bundle_static/Cargo.toml +++ b/libsql-sqlite3/ext/crr/rs/bundle_static/Cargo.toml @@ -20,6 +20,7 @@ panic = "abort" panic = "abort" [features] +libsql = ["crsql_bundle/libsql"] test = [ "crsql_bundle/test" ] diff --git a/libsql-sqlite3/ext/crr/rs/core/Cargo.toml b/libsql-sqlite3/ext/crr/rs/core/Cargo.toml index 73a9eba696..c6464f5aa4 100644 --- a/libsql-sqlite3/ext/crr/rs/core/Cargo.toml +++ b/libsql-sqlite3/ext/crr/rs/core/Cargo.toml @@ -26,6 +26,7 @@ panic = "abort" [features] test = [] +libsql = [] loadable_extension = ["sqlite_nostd/loadable_extension"] static = ["sqlite_nostd/static"] omit_load_extension = ["sqlite_nostd/omit_load_extension"] diff --git a/libsql-sqlite3/ext/crr/rs/core/src/changes_vtab_write.rs b/libsql-sqlite3/ext/crr/rs/core/src/changes_vtab_write.rs index 3da68cd1c7..c9a28c2492 100644 --- a/libsql-sqlite3/ext/crr/rs/core/src/changes_vtab_write.rs +++ b/libsql-sqlite3/ext/crr/rs/core/src/changes_vtab_write.rs @@ -28,12 +28,14 @@ use crate::util::slab_rowid; */ fn did_cid_win( db: *mut sqlite3, + ext_data: *mut crsql_ExtData, insert_tbl: &str, tbl_info: &TableInfo, unpacked_pks: &Vec, key: sqlite::int64, - col_name: &str, insert_val: *mut sqlite::value, + insert_site_id: &[u8], + col_name: &str, col_version: sqlite::int64, errmsg: *mut *mut c_char, ) -> Result { @@ -76,9 +78,19 @@ fn did_cid_win( } // versions are equal - // need to pull the current value and compare - // we could compare on site_id if we can guarantee site_id is always provided. - // would be slightly more performant.. + // need to compare site ids + let ret = unsafe { + let my_site_id = core::slice::from_raw_parts((*ext_data).siteId, 16); + insert_site_id.cmp(my_site_id) as c_int + }; + + // site id lost. + if ret <= 0 { + return Ok(false); + } + + // site id won + // last thing, compare values to ensure we're not changing state on equal values let col_val_stmt_ref = tbl_info.get_col_value_stmt(db, col_name)?; let col_val_stmt = col_val_stmt_ref.as_ref().ok_or(ResultCode::ERROR)?; @@ -94,7 +106,9 @@ fn did_cid_win( let local_value = col_val_stmt.column_value(0)?; let ret = crsql_compare_sqlite_values(insert_val, local_value); reset_cached_stmt(col_val_stmt.stmt)?; - return Ok(ret > 0); + // insert site id won and values differ. We should take the update. + // if values are the same (ret == 0) then we return false and do not take the update + return Ok(ret != 0); } _ => { // ResultCode::DONE would happen if clock values exist but actual values are missing. @@ -585,12 +599,14 @@ unsafe fn merge_insert( || !row_exists_locally || did_cid_win( db, + (*tab).pExtData, insert_tbl, &tbl_info, &unpacked_pks, key, - insert_col, insert_val, + insert_site_id, + insert_col, insert_col_vrsn, errmsg, )?; diff --git a/libsql-sqlite3/ext/crr/rs/core/src/create_cl_set_vtab.rs b/libsql-sqlite3/ext/crr/rs/core/src/create_cl_set_vtab.rs index 17ae78bc10..1fc5c0a900 100644 --- a/libsql-sqlite3/ext/crr/rs/core/src/create_cl_set_vtab.rs +++ b/libsql-sqlite3/ext/crr/rs/core/src/create_cl_set_vtab.rs @@ -256,6 +256,8 @@ static MODULE: sqlite_nostd::module = sqlite_nostd::module { xRelease: None, xRollbackTo: None, xShadowName: None, + #[cfg(feature = "libsql")] + xPreparedSql: None, }; pub fn create_module(db: *mut sqlite::sqlite3) -> Result { diff --git a/libsql-sqlite3/ext/crr/rs/core/src/unpack_columns_vtab.rs b/libsql-sqlite3/ext/crr/rs/core/src/unpack_columns_vtab.rs index e4b3e28e43..3b067f2085 100644 --- a/libsql-sqlite3/ext/crr/rs/core/src/unpack_columns_vtab.rs +++ b/libsql-sqlite3/ext/crr/rs/core/src/unpack_columns_vtab.rs @@ -253,6 +253,8 @@ static MODULE: sqlite_nostd::module = sqlite_nostd::module { xRelease: None, xRollbackTo: None, xShadowName: None, + #[cfg(feature = "libsql")] + xPreparedSql: None, }; /** diff --git a/libsql-sqlite3/ext/crr/rs/integration-check/Cargo.toml b/libsql-sqlite3/ext/crr/rs/integration-check/Cargo.toml deleted file mode 100644 index ad311303f4..0000000000 --- a/libsql-sqlite3/ext/crr/rs/integration-check/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -name = "integration-check" -version = "0.0.1" -edition = "2021" -authors = ["Matt Wonlaw"] -description = "rs integration check for crsqlite" -keywords = ["sqlite"] -license = "Apache-2.0" - -[dependencies] -sqlite_nostd = { path="../sqlite-rs-embedded/sqlite_nostd", features=["static"] } -crsql_core = { path="../core" } -integration_utils = { path="../integration-utils" } - -[build-dependencies] -cc = "1.0.79" diff --git a/libsql-sqlite3/ext/crr/rs/integration_check/Cargo.toml b/libsql-sqlite3/ext/crr/rs/integration_check/Cargo.toml index f53e5af790..c40c89a9c7 100644 --- a/libsql-sqlite3/ext/crr/rs/integration_check/Cargo.toml +++ b/libsql-sqlite3/ext/crr/rs/integration_check/Cargo.toml @@ -21,5 +21,6 @@ libc-print = "0.1.22" cc = "1.0" [features] +libsql = ["crsql_bundle/libsql"] static = [] omit_load_extension = [] diff --git a/libsql-sqlite3/ext/crr/rs/sqlite-rs-embedded/sqlite_nostd/src/nostd.rs b/libsql-sqlite3/ext/crr/rs/sqlite-rs-embedded/sqlite_nostd/src/nostd.rs index 7a051874f6..0299b53da0 100644 --- a/libsql-sqlite3/ext/crr/rs/sqlite-rs-embedded/sqlite_nostd/src/nostd.rs +++ b/libsql-sqlite3/ext/crr/rs/sqlite-rs-embedded/sqlite_nostd/src/nostd.rs @@ -806,6 +806,7 @@ pub trait Context { fn result_error_code(&self, code: ResultCode); fn result_value(&self, value: *mut value); fn result_double(&self, value: f64); + fn result_int(&self, value: i32); fn result_int64(&self, value: int64); fn result_null(&self); fn db_handle(&self) -> *mut sqlite3; @@ -906,6 +907,11 @@ impl Context for *mut context { result_int64(*self, value); } + #[inline] + fn result_int(&self, value: i32) { + result_int(*self, value); + } + #[inline] fn db_handle(&self) -> *mut sqlite3 { context_db_handle(*self) diff --git a/libsql-sqlite3/ext/crr/src/changes-vtab.c b/libsql-sqlite3/ext/crr/src/changes-vtab.c index f088a664de..923f683918 100644 --- a/libsql-sqlite3/ext/crr/src/changes-vtab.c +++ b/libsql-sqlite3/ext/crr/src/changes-vtab.c @@ -176,4 +176,9 @@ sqlite3_module crsql_changesModule = { /* xSavepoint */ 0, /* xRelease */ 0, /* xRollbackTo */ 0, - /* xShadowName */ 0}; + /* xShadowName */ 0 +#ifdef LIBSQL + , + /* xPreparedSql */ 0 +#endif +}; diff --git a/libsql-sqlite3/ext/crr/src/crsqlite.c b/libsql-sqlite3/ext/crr/src/crsqlite.c index fd7197df6c..b764410f4a 100644 --- a/libsql-sqlite3/ext/crr/src/crsqlite.c +++ b/libsql-sqlite3/ext/crr/src/crsqlite.c @@ -287,6 +287,13 @@ static void rollbackHook(void *pUserData) { pExtData->readDbVersionThisTx = 0; } +#ifdef LIBSQL +static void closeHook(void *pUserData, sqlite3 *db) { + crsql_ExtData *pExtData = (crsql_ExtData *)pUserData; + crsql_finalize(pExtData); +} +#endif + int sqlite3_crsqlrustbundle_init(sqlite3 *db, char **pzErrMsg, const sqlite3_api_routines *pApi); @@ -294,10 +301,18 @@ int sqlite3_crsqlrustbundle_init(sqlite3 *db, char **pzErrMsg, __declspec(dllexport) #endif int sqlite3_crsqlite_init(sqlite3 *db, char **pzErrMsg, - const sqlite3_api_routines *pApi) { + const sqlite3_api_routines *pApi +#ifdef LIBSQL + , + const libsql_api_routines *pLibsqlApi +#endif + ) { int rc = SQLITE_OK; SQLITE_EXTENSION_INIT2(pApi); +#ifdef LIBSQL + LIBSQL_EXTENSION_INIT2(pLibsqlApi); +#endif // TODO: should be moved lower once we finish migrating to rust. // RN it is safe here since the rust bundle init is largely just reigstering @@ -432,6 +447,9 @@ __declspec(dllexport) } if (rc == SQLITE_OK) { +#ifdef LIBSQL + libsql_close_hook(db, closeHook, pExtData); +#endif // TODO: get the prior callback so we can call it rather than replace // it? sqlite3_commit_hook(db, commitHook, pExtData); diff --git a/libsql-sqlite3/ext/crr/src/crsqlite.test.c b/libsql-sqlite3/ext/crr/src/crsqlite.test.c index 539322255e..fe3c49df8d 100644 --- a/libsql-sqlite3/ext/crr/src/crsqlite.test.c +++ b/libsql-sqlite3/ext/crr/src/crsqlite.test.c @@ -440,6 +440,20 @@ static sqlite3_int64 getDbVersion(sqlite3 *db) { return db2v; } +static void *getSiteId(sqlite3 *db) { + sqlite3_stmt *pStmt = 0; + int rc = sqlite3_prepare_v2(db, "SELECT crsql_site_id()", -1, &pStmt, 0); + if (rc != SQLITE_OK) { + return 0; + } + + sqlite3_step(pStmt); + void *site = sqlite3_column_blob(pStmt, 0); + sqlite3_finalize(pStmt); + + return site; +} + static void testLamportCondition() { printf("LamportCondition\n"); // syncing from A -> B, while no changes happen on B, moves up @@ -511,6 +525,17 @@ static void noopsDoNotMoveClocks() { rc += sqlite3_open(":memory:", &db1); rc += sqlite3_open(":memory:", &db2); + void *db1SiteId = getSiteId(db1); + void *db2SiteId = getSiteId(db2); + int cmp = memcmp(db1SiteId, db2SiteId, 16); + if (cmp > 0) { + sqlite3 *temp = db1; + db1 = db2; + db2 = temp; + } + + // swap dbs based on site id compare to make it a noop. + rc += sqlite3_exec( db1, "CREATE TABLE \"hoot\" (\"a\", \"b\" primary key not null, \"c\")", 0, 0, 0); @@ -545,6 +570,10 @@ static void noopsDoNotMoveClocks() { sqlite3_int64 db1vPost = getDbVersion(db1); sqlite3_int64 db2vPost = getDbVersion(db2); + // TODO: we still need to compare values so as not to bump the db_version + // forward on a no-difference + // this fails sometimes because site id winning. + printf("db1 pre: %lld db2 post: %lld", db1vPre, db2vPost); assert(db1vPre == db2vPost); assert(db1vPre == db1vPost); diff --git a/libsql-sqlite3/ext/crr/src/rows-impacted.test.c b/libsql-sqlite3/ext/crr/src/rows-impacted.test.c index 874d75bb6a..ad3e9d0d7a 100644 --- a/libsql-sqlite3/ext/crr/src/rows-impacted.test.c +++ b/libsql-sqlite3/ext/crr/src/rows-impacted.test.c @@ -314,7 +314,7 @@ static void testCreateThatDoesNotChangeAnything() { printf("\t\e[0;32mSuccess\e[0m\n"); } -static void testValueWin() { +static void testValueWouldWinButSiteIdLoses() { printf("ValueWin\n"); int rc = SQLITE_OK; char *err = 0; @@ -326,10 +326,37 @@ static void testValueWin() { rc = sqlite3_exec(db, "BEGIN", 0, 0, 0); rc += sqlite3_exec(db, "INSERT INTO crsql_changes VALUES ('foo', X'010901', 'b', " - "3, 1, 1, NULL, 1, 1)", + "3, 1, 1, X'00000000000000000000000000000000', 1, 1)", 0, 0, &err); sqlite3_prepare_v2(db, "SELECT crsql_rows_impacted()", -1, &pStmt, 0); sqlite3_step(pStmt); + // value is greater but site id lower, a loss and now rows changed. + assert(sqlite3_column_int(pStmt, 0) == 0); + sqlite3_finalize(pStmt); + rc += sqlite3_exec(db, "COMMIT", 0, 0, 0); + assert(rc == SQLITE_OK); + + crsql_close(db); + printf("\t\e[0;32mSuccess\e[0m\n"); +} + +static void testSiteIdWin() { + printf("SiteIdWin\n"); + int rc = SQLITE_OK; + char *err = 0; + sqlite3 *db = createDb(); + sqlite3_stmt *pStmt = 0; + + rc = sqlite3_exec(db, "INSERT INTO foo VALUES (1, 2)", 0, 0, 0); + + rc = sqlite3_exec(db, "BEGIN", 0, 0, 0); + rc += sqlite3_exec(db, + "INSERT INTO crsql_changes VALUES ('foo', X'010901', 'b', " + "3, 1, 1, X'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF', 1, 1)", + 0, 0, &err); + sqlite3_prepare_v2(db, "SELECT crsql_rows_impacted()", -1, &pStmt, 0); + sqlite3_step(pStmt); + // site id is larger, a win assert(sqlite3_column_int(pStmt, 0) == 1); sqlite3_finalize(pStmt); rc += sqlite3_exec(db, "COMMIT", 0, 0, 0); @@ -374,7 +401,8 @@ void rowsImpactedTestSuite() { testUpdateThatDoesNotChangeAnything(); testDeleteThatDoesNotChangeAnything(); testCreateThatDoesNotChangeAnything(); - testValueWin(); + testValueWouldWinButSiteIdLoses(); + testSiteIdWin(); testClockWin(); testDelete(); } \ No newline at end of file