Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to compare site_id when values are equal to determine ultimate winner #415

Merged
merged 9 commits into from
Dec 27, 2023
4 changes: 2 additions & 2 deletions core/rs/bundle/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions core/rs/core/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions core/rs/core/src/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ pub enum ChangeRowType {
}

#[repr(C)]
#[derive(Debug, Copy, Clone)]
#[allow(non_snake_case, non_camel_case_types)]
#[derive(Debug, Copy, Clone)]
pub struct crsql_ExtData {
pub pPragmaSchemaVersionStmt: *mut sqlite::stmt,
pub pPragmaDataVersionStmt: *mut sqlite::stmt,
Expand All @@ -67,6 +67,7 @@ pub struct crsql_ExtData {
pub pSetSiteIdOrdinalStmt: *mut sqlite::stmt,
pub pSelectSiteIdOrdinalStmt: *mut sqlite::stmt,
pub pSelectClockTablesStmt: *mut sqlite::stmt,
pub mergeEqualValues: ::core::ffi::c_int,
}

#[repr(C)]
Expand Down Expand Up @@ -262,7 +263,7 @@ fn bindgen_test_layout_crsql_ExtData() {
let ptr = UNINIT.as_ptr();
assert_eq!(
::core::mem::size_of::<crsql_ExtData>(),
128usize,
136usize,
concat!("Size of: ", stringify!(crsql_ExtData))
);
assert_eq!(
Expand Down Expand Up @@ -452,4 +453,14 @@ fn bindgen_test_layout_crsql_ExtData() {
stringify!(pSelectClockTablesStmt)
)
);
assert_eq!(
unsafe { ::core::ptr::addr_of!((*ptr).mergeEqualValues) as usize - ptr as usize },
128usize,
concat!(
"Offset of field: ",
stringify!(crsql_ExtData),
"::",
stringify!(mergeEqualValues)
)
);
}
21 changes: 15 additions & 6 deletions core/rs/core/src/changes_vtab_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,21 @@ use crate::util::slab_rowid;

/**
* did_cid_win does not take into account the causal length.
* The expectation is that all cuasal length concerns have already been handle
* The expectation is that all causal length concerns have already been handle
* via:
* - early return because insert_cl < local_cl
* - automatic win because insert_cl > local_cl
* - come here to did_cid_win iff insert_cl = local_cl
* - come here to did_cid_win if insert_cl = local_cl
*/
fn did_cid_win(
db: *mut sqlite3,
ext_data: *mut crsql_ExtData,
insert_tbl: &str,
tbl_info: &TableInfo,
unpacked_pks: &Vec<ColumnValue>,
key: sqlite::int64,
insert_val: *mut sqlite::value,
insert_site_id: &[u8],
col_name: &str,
col_version: sqlite::int64,
errmsg: *mut *mut c_char,
Expand Down Expand Up @@ -89,10 +91,15 @@ fn did_cid_win(
match step_result {
Ok(ResultCode::ROW) => {
let local_value = col_val_stmt.column_value(0)?;
let ret = crsql_compare_sqlite_values(insert_val, local_value);
let mut ret = crsql_compare_sqlite_values(insert_val, local_value);
reset_cached_stmt(col_val_stmt.stmt)?;
// value won, take value
// if values are the same (ret == 0) then we return false and do not take the update
if ret == 0 && unsafe { (*ext_data).mergeEqualValues == 1 } {
// values are the same (ret == 0) and the option to tie break on site_id is true
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
};
}
return Ok(ret > 0);
}
_ => {
Expand Down Expand Up @@ -586,17 +593,19 @@ unsafe fn merge_insert(
|| !row_exists_locally
|| did_cid_win(
db,
(*tab).pExtData,
insert_tbl,
&tbl_info,
&unpacked_pks,
key,
insert_val,
insert_site_id,
insert_col,
insert_col_vrsn,
errmsg,
)?;

if does_cid_win == false {
if !does_cid_win {
// doesCidWin == 0? compared against our clocks, nothing wins. OK and
// Done.
return Ok(ResultCode::OK);
Expand Down
85 changes: 85 additions & 0 deletions core/rs/core/src/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use alloc::format;

use sqlite::{Connection, Context};
use sqlite_nostd as sqlite;
use sqlite_nostd::{ResultCode, Value};

use crate::c::crsql_ExtData;

pub const MERGE_EQUAL_VALUES: &str = "merge-equal-values";

pub extern "C" fn crsql_config_set(
ctx: *mut sqlite::context,
argc: i32,
argv: *mut *mut sqlite::value,
) {
let args = sqlite::args!(argc, argv);

let name = args[0].text();

let value = match name {
MERGE_EQUAL_VALUES => {
let value = args[1];
let ext_data = ctx.user_data() as *mut crsql_ExtData;
unsafe { (*ext_data).mergeEqualValues = value.int() };
value
}
_ => {
ctx.result_error("Unknown setting name");
ctx.result_error_code(ResultCode::ERROR);
return;
}
};

let db = ctx.db_handle();
match insert_config_setting(db, name, value) {
Ok(value) => {
ctx.result_value(value);
}
Err(rc) => {
ctx.result_error("Could not persist config in database");
ctx.result_error_code(rc);
return;
}
}
}

fn insert_config_setting(
db: *mut sqlite_nostd::sqlite3,
name: &str,
value: *mut sqlite::value,
) -> Result<*mut sqlite::value, ResultCode> {
let stmt =
db.prepare_v2("INSERT OR REPLACE INTO crsql_master VALUES (?, ?) RETURNING value")?;

stmt.bind_text(1, &format!("config.{name}"), sqlite::Destructor::TRANSIENT)?;
stmt.bind_value(2, value)?;

if let ResultCode::ROW = stmt.step()? {
stmt.column_value(0)
} else {
Err(ResultCode::ERROR)
}
}

pub extern "C" fn crsql_config_get(
ctx: *mut sqlite::context,
argc: i32,
argv: *mut *mut sqlite::value,
) {
let args = sqlite::args!(argc, argv);

let name = args[0].text();

match name {
MERGE_EQUAL_VALUES => {
let ext_data = ctx.user_data() as *mut crsql_ExtData;
ctx.result_int(unsafe { (*ext_data).mergeEqualValues });
}
_ => {
ctx.result_error("Unknown setting name");
ctx.result_error_code(ResultCode::ERROR);
return;
}
}
}
38 changes: 37 additions & 1 deletion core/rs/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod changes_vtab;
mod changes_vtab_read;
mod changes_vtab_write;
mod compare_values;
mod config;
mod consts;
mod create_cl_set_vtab;
mod create_crr;
Expand Down Expand Up @@ -53,6 +54,7 @@ use alter::crsql_compact_post_alter;
use automigrate::*;
use backfill::*;
use c::{crsql_freeExtData, crsql_newExtData};
use config::{crsql_config_get, crsql_config_set};
use core::ffi::{c_int, c_void, CStr};
use create_crr::create_crr;
use db_version::{crsql_fill_db_version_if_needed, crsql_next_db_version};
Expand Down Expand Up @@ -212,7 +214,7 @@ pub extern "C" fn sqlite3_crsqlcore_init(

let ext_data = unsafe { crsql_newExtData(db, site_id_buffer as *mut c_char) };
if ext_data.is_null() {
sqlite::free(site_id_buffer as *mut c_void);
// no need to free the site id buffer here, this is cleaned up already.
return null_mut();
}

Expand Down Expand Up @@ -471,6 +473,40 @@ pub extern "C" fn sqlite3_crsqlcore_init(
return null_mut();
}

let rc = db
.create_function_v2(
"crsql_config_set",
2,
sqlite::UTF8,
Some(ext_data as *mut c_void),
Some(crsql_config_set),
None,
None,
None,
)
.unwrap_or(sqlite::ResultCode::ERROR);
if rc != ResultCode::OK {
unsafe { crsql_freeExtData(ext_data) };
return null_mut();
}

let rc = db
.create_function_v2(
"crsql_config_get",
1,
sqlite::UTF8 | sqlite::INNOCUOUS | sqlite::DETERMINISTIC,
Some(ext_data as *mut c_void),
Some(crsql_config_get),
None,
None,
None,
)
.unwrap_or(sqlite::ResultCode::ERROR);
if rc != ResultCode::OK {
unsafe { crsql_freeExtData(ext_data) };
return null_mut();
}

return ext_data as *mut c_void;
}

Expand Down
2 changes: 1 addition & 1 deletion core/rs/core/src/sha.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
// The sha of the commit that this version of crsqlite was built from.
pub const SHA: &'static str = "";
pub const SHA: &'static str = "3a01980562615001d765eec61e3ed58147a93f93";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I guess I should ignore this file and only generate it at build time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, possibly yes. I didn't notice it.

39 changes: 25 additions & 14 deletions core/rs/fractindex-core/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading