From 37cdb5a5e3c0078c11fd974629c65c48cc4f2a9d Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Wed, 27 Nov 2024 14:43:51 -0500 Subject: [PATCH 1/7] make a fast crsql_commit_alter by specifying if it's a non-destructive change and only managing trigger changes --- core/rs/bundle_static/Cargo.lock | 10 ++ core/rs/core/Cargo.lock | 10 ++ core/rs/core/Cargo.toml | 1 + core/rs/core/src/alter.rs | 9 ++ core/rs/core/src/backfill.rs | 10 +- core/rs/core/src/create_cl_set_vtab.rs | 14 ++- core/rs/core/src/create_crr.rs | 47 ++++++++- core/rs/core/src/lib.rs | 131 ++++++++++++++++++++----- core/rs/core/src/tableinfo.rs | 10 +- core/rs/core/src/triggers.rs | 27 +++++ 10 files changed, 237 insertions(+), 32 deletions(-) diff --git a/core/rs/bundle_static/Cargo.lock b/core/rs/bundle_static/Cargo.lock index a0858632d..909df5a63 100644 --- a/core/rs/bundle_static/Cargo.lock +++ b/core/rs/bundle_static/Cargo.lock @@ -108,6 +108,7 @@ name = "crsql_core" version = "0.1.0" dependencies = [ "bytes", + "libc-print", "num-derive", "num-traits", "sqlite_nostd", @@ -180,6 +181,15 @@ version = "0.2.148" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" +[[package]] +name = "libc-print" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4a660208db49e35faf57b37484350f1a61072f2a5becf0592af6015d9ddd4b0" +dependencies = [ + "libc", +] + [[package]] name = "libloading" version = "0.7.4" diff --git a/core/rs/core/Cargo.lock b/core/rs/core/Cargo.lock index d2006ae23..dc45e3470 100644 --- a/core/rs/core/Cargo.lock +++ b/core/rs/core/Cargo.lock @@ -74,6 +74,7 @@ name = "crsql_core" version = "0.1.0" dependencies = [ "bytes", + "libc-print", "num-derive", "num-traits", "sqlite_nostd", @@ -109,6 +110,15 @@ version = "0.2.151" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "302d7ab3130588088d277783b1e2d2e10c9e9e4a16dd9050e6ec93fb3e7048f4" +[[package]] +name = "libc-print" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4a660208db49e35faf57b37484350f1a61072f2a5becf0592af6015d9ddd4b0" +dependencies = [ + "libc", +] + [[package]] name = "libloading" version = "0.7.4" diff --git a/core/rs/core/Cargo.toml b/core/rs/core/Cargo.toml index c6464f5aa..13ef9b804 100644 --- a/core/rs/core/Cargo.toml +++ b/core/rs/core/Cargo.toml @@ -15,6 +15,7 @@ sqlite_nostd = { path="../sqlite-rs-embedded/sqlite_nostd" } bytes = { version = "1.5", default-features = false } num-traits = { version = "0.2.17", default-features = false } num-derive = "0.4.1" +libc-print = "0.1.23" [dev-dependencies] diff --git a/core/rs/core/src/alter.rs b/core/rs/core/src/alter.rs index baf131208..398a88470 100644 --- a/core/rs/core/src/alter.rs +++ b/core/rs/core/src/alter.rs @@ -33,10 +33,12 @@ unsafe fn compact_post_alter( errmsg: *mut *mut c_char, ) -> Result { let tbl_name_str = CStr::from_ptr(tbl_name).to_str()?; + libc_print::libc_println!("fill_db_version_if_needed"); fill_db_version_if_needed(db, ext_data).or_else(|msg| { errmsg.set(&msg); Err(ResultCode::ERROR) })?; + libc_print::libc_println!("DONE fill_db_version_if_needed"); let current_db_version = (*ext_data).dbVersion; // If primary key columns change (in the schema) @@ -63,6 +65,7 @@ unsafe fn compact_post_alter( drop(stmt); if pk_diff > 0 { + libc_print::libc_println!("dropping table, pk diff > 0!"); // drop the clock table so we can re-create it db.exec_safe(&format!( "DROP TABLE \"{table_name}__crsql_clock\"; @@ -82,7 +85,9 @@ unsafe fn compact_post_alter( tbl_name_val = crate::util::escape_ident_as_value(tbl_name_str), cl_sentinel = crate::c::DELETE_SENTINEL, ); + libc_print::libc_println!("compacting entries without a column... {}", sql); db.exec_safe(&sql)?; + libc_print::libc_println!("DONE compacting entries without a column..."); // Next delete entries that no longer have a row but keeping tombstones // TODO: if we move the sentinel metadata to the lookaside this becomes much simpler @@ -128,7 +133,9 @@ unsafe fn compact_post_alter( tbl_name = crate::util::escape_ident(tbl_name_str) ) ); + libc_print::libc_println!("deleting entries no longer have rows {}", sql); db.exec_safe(&sql)?; + libc_print::libc_println!("DONE deleteing entries no longer have rows"); // now delete pk lookasides that no longer map to anything in the clock tables let sql = format!( @@ -137,7 +144,9 @@ unsafe fn compact_post_alter( )", tbl_name = crate::util::escape_ident(tbl_name_str), ); + libc_print::libc_println!("deleting extra pk lookasides {}", sql); db.exec_safe(&sql)?; + libc_print::libc_println!("DONE deleting extra pk lookasides"); } let stmt = db.prepare_v2( diff --git a/core/rs/core/src/backfill.rs b/core/rs/core/src/backfill.rs index 8787739da..850dc0960 100644 --- a/core/rs/core/src/backfill.rs +++ b/core/rs/core/src/backfill.rs @@ -1,6 +1,6 @@ use sqlite_nostd::{sqlite3, Connection, Destructor, ManagedStmt, ResultCode}; extern crate alloc; -use crate::tableinfo::ColumnInfo; +use crate::tableinfo::{ColumnInfo, TableInfo}; use crate::util::get_dflt_value; use alloc::format; use alloc::string::String; @@ -12,10 +12,12 @@ use sqlite_nostd as sqlite; */ pub fn backfill_table( db: *mut sqlite3, + curr_table_info: Option<&TableInfo>, table: &str, pk_cols: &Vec, non_pk_cols: &Vec, is_commit_alter: bool, + pks_changed: bool, no_tx: bool, ) -> Result { if !no_tx { @@ -35,6 +37,7 @@ pub fn backfill_table( let stmt = db.prepare_v2(&sql); let non_pk_cols_refs = non_pk_cols.iter().collect::>(); + libc_print::libc_println!("creating clock rows"); let result = match stmt { Ok(stmt) => create_clock_rows_from_stmt( stmt, @@ -46,6 +49,7 @@ pub fn backfill_table( ), Err(e) => Err(e), }; + libc_print::libc_println!("DONE creating clock rows"); if let Err(e) = result { if !no_tx { @@ -55,6 +59,7 @@ pub fn backfill_table( return Err(e); } + libc_print::libc_println!("backfilling missing columns"); if let Err(e) = backfill_missing_columns(db, table, pk_cols, non_pk_cols, is_commit_alter) { if !no_tx { db.exec_safe("ROLLBACK")?; @@ -62,6 +67,7 @@ pub fn backfill_table( return Err(e); } + libc_print::libc_println!("DONE backfilling missing columns"); if !no_tx { db.exec_safe("RELEASE backfill") @@ -184,7 +190,9 @@ fn backfill_missing_columns( is_commit_alter: bool, ) -> Result { for non_pk_col in non_pk_cols { + libc_print::libc_println!("backfilling column {}", non_pk_col.name); fill_column(db, table, pk_cols, &non_pk_col, is_commit_alter)?; + libc_print::libc_println!("DONE backfilling column {}", non_pk_col.name); } Ok(ResultCode::OK) diff --git a/core/rs/core/src/create_cl_set_vtab.rs b/core/rs/core/src/create_cl_set_vtab.rs index 34fd16839..5957e4bc3 100644 --- a/core/rs/core/src/create_cl_set_vtab.rs +++ b/core/rs/core/src/create_cl_set_vtab.rs @@ -1,12 +1,16 @@ extern crate alloc; use core::ffi::{c_char, c_int, c_void}; +use core::mem; use crate::alloc::borrow::ToOwned; +use crate::c::crsql_Changes_vtab; use crate::create_crr::create_crr; +use crate::tableinfo::TableInfo; use alloc::boxed::Box; use alloc::format; use alloc::string::String; +use alloc::vec::Vec; use sqlite::{sqlite3, Connection, CursorRef, StrRef, VTabArgs, VTabRef}; use sqlite_nostd as sqlite; use sqlite_nostd::ResultCode; @@ -69,7 +73,15 @@ fn create_impl( create_clset_storage(db, &vtab_args, err)?; let schema = vtab_args.database_name; let table = base_name_from_virtual_name(vtab_args.table_name); - create_crr(db, schema, table, false, true, err) + + let tab = vtab.cast::(); + let table_infos = unsafe { + mem::ManuallyDrop::new(Box::from_raw( + (*(*tab).pExtData).tableInfos as *mut Vec, + )) + }; + + create_crr(db, &table_infos, schema, table, false, true, err) } fn create_clset_storage( diff --git a/core/rs/core/src/create_crr.rs b/core/rs/core/src/create_crr.rs index 311a89367..245b60dcf 100644 --- a/core/rs/core/src/create_crr.rs +++ b/core/rs/core/src/create_crr.rs @@ -1,9 +1,10 @@ +use alloc::vec::Vec; use core::ffi::c_char; use sqlite_nostd as sqlite; use sqlite_nostd::ResultCode; use crate::bootstrap::create_clock_table; -use crate::tableinfo::{is_table_compatible, pull_table_info}; +use crate::tableinfo::{is_table_compatible, pull_table_info, TableInfo}; use crate::triggers::create_triggers; use crate::{backfill_table, is_crr, remove_crr_triggers_if_exist}; @@ -13,6 +14,7 @@ use crate::{backfill_table, is_crr, remove_crr_triggers_if_exist}; */ pub fn create_crr( db: *mut sqlite::sqlite3, + table_infos: &Vec, _schema: &str, table: &str, is_commit_alter: bool, @@ -32,18 +34,55 @@ pub fn create_crr( // when upgrading stuff to CRRs let table_info = pull_table_info(db, table, err)?; - create_clock_table(db, &table_info, err)?; - remove_crr_triggers_if_exist(db, table)?; - create_triggers(db, &table_info, err)?; + let curr_table_info = table_infos.iter().find(|t| t.tbl_name == table); + if curr_table_info.is_none() { + libc_print::libc_println!("creating clock table"); + create_clock_table(db, &table_info, err)?; + libc_print::libc_println!("DONE creating clock table"); + } else { + libc_print::libc_println!("removing triggers if exist"); + remove_crr_triggers_if_exist(db, table)?; + libc_print::libc_println!("DONE removing triggers if exist"); + } + + let pks_changed = if let Some(curr_table_info) = curr_table_info { + // PAINFUL, but cheap in comparison to what it prevents + let curr_contains_all = curr_table_info.pks.iter().all(|curr_t| { + table_info + .pks + .iter() + .any(|t| curr_t.name == t.name && curr_t.cid == t.cid) + }); + let new_contains_all = table_info.pks.iter().all(|new_t| { + curr_table_info + .pks + .iter() + .any(|t| new_t.name == t.name && new_t.cid == t.cid) + }); + curr_contains_all && new_contains_all + } else { + true + }; + + if pks_changed { + libc_print::libc_println!("creating triggers"); + create_triggers(db, &table_info, err)?; + libc_print::libc_println!("DONE creating triggers"); + } + + libc_print::libc_println!("backfilling table"); backfill_table( db, + curr_table_info, table, &table_info.pks, &table_info.non_pks, is_commit_alter, + pks_changed, no_tx, )?; + libc_print::libc_println!("DONE backfilling table"); Ok(ResultCode::OK) } diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 72d6906a4..474fabb0f 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -50,6 +50,7 @@ use core::ffi::c_char; use core::mem; use core::ptr::null_mut; extern crate alloc; +use alloc::boxed::Box; use alter::crsql_compact_post_alter; use automigrate::*; use backfill::*; @@ -65,8 +66,11 @@ use local_writes::after_update::x_crsql_after_update; use sqlite::{Destructor, ResultCode}; use sqlite_nostd as sqlite; use sqlite_nostd::{Connection, Context, Value}; -use tableinfo::is_table_compatible; +use tableinfo::{ + crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info, TableInfo, +}; use teardown::*; +use triggers::recreate_update_triggers; pub extern "C" fn crsql_as_table( ctx: *mut sqlite::context, @@ -567,8 +571,15 @@ unsafe extern "C" fn x_crsql_as_crr( ctx.result_error("failed to start as_crr savepoint"); return; } + + let ext_data = ctx.user_data() as *mut c::crsql_ExtData; + let table_infos = mem::ManuallyDrop::new(Box::from_raw( + (*ext_data).tableInfos as *mut alloc::vec::Vec, + )); + let rc = crsql_create_crr( db, + &table_infos, schema_name.as_ptr() as *const c_char, table_name.as_ptr() as *const c_char, 0, @@ -650,34 +661,92 @@ unsafe extern "C" fn x_crsql_commit_alter( } let args = sqlite::args!(argc, argv); - let (schema_name, table_name) = if argc == 2 { + let (schema_name, table_name) = if argc >= 2 { (args[0].text(), args[1].text()) } else { ("main", args[0].text()) }; + let non_destructive = if argc >= 3 { args[2].int() == 1 } else { false }; + + libc_print::libc_println!("non-destructive? {}", non_destructive); + let ext_data = ctx.user_data() as *mut c::crsql_ExtData; let mut err_msg = null_mut(); let db = ctx.db_handle(); - let rc = crsql_compact_post_alter( - db, - table_name.as_ptr() as *const c_char, - ext_data, - &mut err_msg as *mut _, - ); - let rc = if rc == ResultCode::OK as c_int { - crsql_create_crr( + let rc = if non_destructive { + match pull_table_info(db, table_name, &mut err_msg as *mut _) { + Ok(table_info) => { + match recreate_update_triggers(db, &table_info, &mut err_msg as *mut _) { + Ok(ResultCode::OK) => { + // need to ensure the right table infos in ext data + crsql_ensure_table_infos_are_up_to_date( + db, + ext_data, + &mut err_msg as *mut _, + ) + } + Ok(rc) | Err(rc) => rc as c_int, + } + } + Err(rc) => rc as c_int, + } + + // recreate_update_triggers(db, &table_info, &mut err_msg as *mut _) + + // crsql_ensure_table_infos_are_up_to_date(db, ext_data, &mut err_msg as *mut _) + } else { + libc_print::libc_println!("compacting post alter"); + let rc = crsql_compact_post_alter( db, - schema_name.as_ptr() as *const c_char, table_name.as_ptr() as *const c_char, - 1, - 0, + ext_data, &mut err_msg as *mut _, - ) - } else { - rc + ); + libc_print::libc_println!("DONE compacting post alter"); + + if rc == ResultCode::OK as c_int { + let table_infos = mem::ManuallyDrop::new(Box::from_raw( + (*ext_data).tableInfos as *mut alloc::vec::Vec, + )); + crsql_create_crr( + db, + &table_infos, + schema_name.as_ptr() as *const c_char, + table_name.as_ptr() as *const c_char, + 1, + 0, + &mut err_msg as *mut _, + ) + } else { + rc + } }; + + // let rc = if rc == ResultCode::OK as c_int { + // let table_infos = mem::ManuallyDrop::new(Box::from_raw( + // (*ext_data).tableInfos as *mut alloc::vec::Vec, + // )); + // crsql_create_crr( + // db, + // &table_infos, + // schema_name.as_ptr() as *const c_char, + // table_name.as_ptr() as *const c_char, + // 1, + // 0, + // &mut err_msg as *mut _, + // ) + // } else { + // rc + // }; + + // let rc = if non_destructive && rc == ResultCode::OK as c_int { + // crsql_ensure_table_infos_are_up_to_date(db, ext_data, &mut err_msg as *mut _) + // } else { + // rc + // }; + let rc = if rc == ResultCode::OK as c_int { db.exec_safe("RELEASE alter_crr") .unwrap_or(ResultCode::ERROR) as c_int @@ -850,6 +919,7 @@ pub extern "C" fn crsql_is_table_compatible( #[no_mangle] pub extern "C" fn crsql_create_crr( db: *mut sqlite::sqlite3, + table_infos: &alloc::vec::Vec, schema: *const c_char, table: *const c_char, is_commit_alter: c_int, @@ -859,11 +929,28 @@ pub extern "C" fn crsql_create_crr( let schema = unsafe { CStr::from_ptr(schema).to_str() }; let table = unsafe { CStr::from_ptr(table).to_str() }; - return match (table, schema) { - (Ok(table), Ok(schema)) => { - create_crr(db, schema, table, is_commit_alter != 0, no_tx != 0, err) - .unwrap_or_else(|err| err) as c_int - } + match (table, schema) { + (Ok(table), Ok(schema)) => create_crr( + db, + table_infos, + schema, + table, + is_commit_alter != 0, + no_tx != 0, + err, + ) + .unwrap_or_else(|err| err) as c_int, _ => ResultCode::NOMEM as c_int, - }; + } +} + +pub enum CommitAlter { + PossiblyDestructive, + NonDestructive, +} + +impl CommitAlter { + pub fn is_destructive(&self) -> bool { + matches!(self, CommitAlter::PossiblyDestructive) + } } diff --git a/core/rs/core/src/tableinfo.rs b/core/rs/core/src/tableinfo.rs index b36ea9a68..aa7352809 100644 --- a/core/rs/core/src/tableinfo.rs +++ b/core/rs/core/src/tableinfo.rs @@ -765,6 +765,7 @@ pub extern "C" fn crsql_ensure_table_infos_are_up_to_date( let mut table_infos = unsafe { Box::from_raw((*ext_data).tableInfos as *mut Vec) }; if schema_changed > 0 || table_infos.len() == 0 { + libc_print::libc_println!("schema changed, pulling tables"); match pull_all_table_infos(db, ext_data, err) { Ok(new_table_infos) => { *table_infos = new_table_infos; @@ -855,14 +856,15 @@ pub fn pull_table_info( let mut cols: Vec = vec![]; while stmt.step()? == ResultCode::ROW { - cols.push(ColumnInfo { + let col = ColumnInfo { name: stmt.column_text(1)?.to_string(), cid: stmt.column_int(0), pk: stmt.column_int(2), curr_value_stmt: RefCell::new(None), merge_insert_stmt: RefCell::new(None), row_patch_data_stmt: RefCell::new(None), - }); + }; + cols.push(col); } if cols.len() != columns_len { @@ -880,7 +882,7 @@ pub fn pull_table_info( let (mut pks, non_pks): (Vec<_>, Vec<_>) = column_infos.into_iter().partition(|x| x.pk > 0); pks.sort_by_key(|x| x.pk); - return Ok(TableInfo { + Ok(TableInfo { tbl_name: table.to_string(), pks, non_pks, @@ -903,7 +905,7 @@ pub fn pull_table_info( mark_locally_created_stmt: RefCell::new(None), mark_locally_updated_stmt: RefCell::new(None), maybe_mark_locally_reinserted_stmt: RefCell::new(None), - }); + }) } pub fn is_table_compatible( diff --git a/core/rs/core/src/triggers.rs b/core/rs/core/src/triggers.rs index 3a3cb0183..941fc754a 100644 --- a/core/rs/core/src/triggers.rs +++ b/core/rs/core/src/triggers.rs @@ -9,6 +9,33 @@ use sqlite_nostd as sqlite; use crate::tableinfo::TableInfo; +pub fn recreate_update_triggers( + db: *mut sqlite3, + table_info: &TableInfo, + err: *mut *mut c_char, +) -> Result { + db.exec_safe(&format!( + "DROP TRIGGER IF EXISTS \"{table}__crsql_utrig\"", + table = crate::util::escape_ident(&table_info.tbl_name) + ))?; + + // get all columns of table + // iterate pk cols + // drop triggers against those pk cols + let stmt = db.prepare_v2("SELECT name FROM pragma_table_info(?) WHERE pk > 0")?; + stmt.bind_text(1, &table_info.tbl_name, sqlite::Destructor::STATIC)?; + while stmt.step()? == ResultCode::ROW { + let col_name = stmt.column_text(0)?; + db.exec_safe(&format!( + "DROP TRIGGER IF EXISTS \"{tbl_name}_{col_name}__crsql_utrig\"", + tbl_name = crate::util::escape_ident(&table_info.tbl_name), + col_name = crate::util::escape_ident(col_name), + ))?; + } + + create_update_trigger(db, table_info, err) +} + pub fn create_triggers( db: *mut sqlite3, table_info: &TableInfo, From ddc6308cf2fb87bc14b31482b899097696285cbb Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Wed, 27 Nov 2024 14:55:42 -0500 Subject: [PATCH 2/7] revert most other unrelated changes --- core/rs/core/src/backfill.rs | 8 +---- core/rs/core/src/create_cl_set_vtab.rs | 9 +----- core/rs/core/src/create_crr.rs | 45 +++----------------------- core/rs/core/src/lib.rs | 28 +++------------- 4 files changed, 11 insertions(+), 79 deletions(-) diff --git a/core/rs/core/src/backfill.rs b/core/rs/core/src/backfill.rs index 850dc0960..7de39e3b0 100644 --- a/core/rs/core/src/backfill.rs +++ b/core/rs/core/src/backfill.rs @@ -1,6 +1,6 @@ use sqlite_nostd::{sqlite3, Connection, Destructor, ManagedStmt, ResultCode}; extern crate alloc; -use crate::tableinfo::{ColumnInfo, TableInfo}; +use crate::tableinfo::ColumnInfo; use crate::util::get_dflt_value; use alloc::format; use alloc::string::String; @@ -12,12 +12,10 @@ use sqlite_nostd as sqlite; */ pub fn backfill_table( db: *mut sqlite3, - curr_table_info: Option<&TableInfo>, table: &str, pk_cols: &Vec, non_pk_cols: &Vec, is_commit_alter: bool, - pks_changed: bool, no_tx: bool, ) -> Result { if !no_tx { @@ -37,7 +35,6 @@ pub fn backfill_table( let stmt = db.prepare_v2(&sql); let non_pk_cols_refs = non_pk_cols.iter().collect::>(); - libc_print::libc_println!("creating clock rows"); let result = match stmt { Ok(stmt) => create_clock_rows_from_stmt( stmt, @@ -49,7 +46,6 @@ pub fn backfill_table( ), Err(e) => Err(e), }; - libc_print::libc_println!("DONE creating clock rows"); if let Err(e) = result { if !no_tx { @@ -59,7 +55,6 @@ pub fn backfill_table( return Err(e); } - libc_print::libc_println!("backfilling missing columns"); if let Err(e) = backfill_missing_columns(db, table, pk_cols, non_pk_cols, is_commit_alter) { if !no_tx { db.exec_safe("ROLLBACK")?; @@ -67,7 +62,6 @@ pub fn backfill_table( return Err(e); } - libc_print::libc_println!("DONE backfilling missing columns"); if !no_tx { db.exec_safe("RELEASE backfill") diff --git a/core/rs/core/src/create_cl_set_vtab.rs b/core/rs/core/src/create_cl_set_vtab.rs index 5957e4bc3..3c07f4656 100644 --- a/core/rs/core/src/create_cl_set_vtab.rs +++ b/core/rs/core/src/create_cl_set_vtab.rs @@ -74,14 +74,7 @@ fn create_impl( let schema = vtab_args.database_name; let table = base_name_from_virtual_name(vtab_args.table_name); - let tab = vtab.cast::(); - let table_infos = unsafe { - mem::ManuallyDrop::new(Box::from_raw( - (*(*tab).pExtData).tableInfos as *mut Vec, - )) - }; - - create_crr(db, &table_infos, schema, table, false, true, err) + create_crr(db, schema, table, false, true, err) } fn create_clset_storage( diff --git a/core/rs/core/src/create_crr.rs b/core/rs/core/src/create_crr.rs index 245b60dcf..6a4bfe050 100644 --- a/core/rs/core/src/create_crr.rs +++ b/core/rs/core/src/create_crr.rs @@ -1,10 +1,9 @@ -use alloc::vec::Vec; use core::ffi::c_char; use sqlite_nostd as sqlite; use sqlite_nostd::ResultCode; use crate::bootstrap::create_clock_table; -use crate::tableinfo::{is_table_compatible, pull_table_info, TableInfo}; +use crate::tableinfo::{is_table_compatible, pull_table_info}; use crate::triggers::create_triggers; use crate::{backfill_table, is_crr, remove_crr_triggers_if_exist}; @@ -14,7 +13,6 @@ use crate::{backfill_table, is_crr, remove_crr_triggers_if_exist}; */ pub fn create_crr( db: *mut sqlite::sqlite3, - table_infos: &Vec, _schema: &str, table: &str, is_commit_alter: bool, @@ -34,52 +32,17 @@ pub fn create_crr( // when upgrading stuff to CRRs let table_info = pull_table_info(db, table, err)?; - let curr_table_info = table_infos.iter().find(|t| t.tbl_name == table); - - if curr_table_info.is_none() { - libc_print::libc_println!("creating clock table"); - create_clock_table(db, &table_info, err)?; - libc_print::libc_println!("DONE creating clock table"); - } else { - libc_print::libc_println!("removing triggers if exist"); - remove_crr_triggers_if_exist(db, table)?; - libc_print::libc_println!("DONE removing triggers if exist"); - } - - let pks_changed = if let Some(curr_table_info) = curr_table_info { - // PAINFUL, but cheap in comparison to what it prevents - let curr_contains_all = curr_table_info.pks.iter().all(|curr_t| { - table_info - .pks - .iter() - .any(|t| curr_t.name == t.name && curr_t.cid == t.cid) - }); - let new_contains_all = table_info.pks.iter().all(|new_t| { - curr_table_info - .pks - .iter() - .any(|t| new_t.name == t.name && new_t.cid == t.cid) - }); - curr_contains_all && new_contains_all - } else { - true - }; - - if pks_changed { - libc_print::libc_println!("creating triggers"); - create_triggers(db, &table_info, err)?; - libc_print::libc_println!("DONE creating triggers"); - } + create_clock_table(db, &table_info, err)?; + remove_crr_triggers_if_exist(db, table)?; + create_triggers(db, &table_info, err)?; libc_print::libc_println!("backfilling table"); backfill_table( db, - curr_table_info, table, &table_info.pks, &table_info.non_pks, is_commit_alter, - pks_changed, no_tx, )?; libc_print::libc_println!("DONE backfilling table"); diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 474fabb0f..646c58821 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -50,7 +50,6 @@ use core::ffi::c_char; use core::mem; use core::ptr::null_mut; extern crate alloc; -use alloc::boxed::Box; use alter::crsql_compact_post_alter; use automigrate::*; use backfill::*; @@ -66,9 +65,7 @@ use local_writes::after_update::x_crsql_after_update; use sqlite::{Destructor, ResultCode}; use sqlite_nostd as sqlite; use sqlite_nostd::{Connection, Context, Value}; -use tableinfo::{ - crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info, TableInfo, -}; +use tableinfo::{crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info}; use teardown::*; use triggers::recreate_update_triggers; @@ -573,13 +570,9 @@ unsafe extern "C" fn x_crsql_as_crr( } let ext_data = ctx.user_data() as *mut c::crsql_ExtData; - let table_infos = mem::ManuallyDrop::new(Box::from_raw( - (*ext_data).tableInfos as *mut alloc::vec::Vec, - )); let rc = crsql_create_crr( db, - &table_infos, schema_name.as_ptr() as *const c_char, table_name.as_ptr() as *const c_char, 0, @@ -707,12 +700,8 @@ unsafe extern "C" fn x_crsql_commit_alter( libc_print::libc_println!("DONE compacting post alter"); if rc == ResultCode::OK as c_int { - let table_infos = mem::ManuallyDrop::new(Box::from_raw( - (*ext_data).tableInfos as *mut alloc::vec::Vec, - )); crsql_create_crr( db, - &table_infos, schema_name.as_ptr() as *const c_char, table_name.as_ptr() as *const c_char, 1, @@ -919,7 +908,6 @@ pub extern "C" fn crsql_is_table_compatible( #[no_mangle] pub extern "C" fn crsql_create_crr( db: *mut sqlite::sqlite3, - table_infos: &alloc::vec::Vec, schema: *const c_char, table: *const c_char, is_commit_alter: c_int, @@ -930,16 +918,10 @@ pub extern "C" fn crsql_create_crr( let table = unsafe { CStr::from_ptr(table).to_str() }; match (table, schema) { - (Ok(table), Ok(schema)) => create_crr( - db, - table_infos, - schema, - table, - is_commit_alter != 0, - no_tx != 0, - err, - ) - .unwrap_or_else(|err| err) as c_int, + (Ok(table), Ok(schema)) => { + create_crr(db, schema, table, is_commit_alter != 0, no_tx != 0, err) + .unwrap_or_else(|err| err) as c_int + } _ => ResultCode::NOMEM as c_int, } } From d36182b4811d1067cc7460acd43782d26c21039e Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Thu, 28 Nov 2024 08:20:36 -0500 Subject: [PATCH 3/7] cleanup, remove printing --- core/rs/core/Cargo.lock | 10 ---------- core/rs/core/Cargo.toml | 1 - core/rs/core/src/alter.rs | 9 --------- core/rs/core/src/backfill.rs | 4 +--- core/rs/core/src/create_crr.rs | 2 -- core/rs/core/src/lib.rs | 31 ------------------------------- core/rs/core/src/tableinfo.rs | 1 - 7 files changed, 1 insertion(+), 57 deletions(-) diff --git a/core/rs/core/Cargo.lock b/core/rs/core/Cargo.lock index dc45e3470..d2006ae23 100644 --- a/core/rs/core/Cargo.lock +++ b/core/rs/core/Cargo.lock @@ -74,7 +74,6 @@ name = "crsql_core" version = "0.1.0" dependencies = [ "bytes", - "libc-print", "num-derive", "num-traits", "sqlite_nostd", @@ -110,15 +109,6 @@ version = "0.2.151" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "302d7ab3130588088d277783b1e2d2e10c9e9e4a16dd9050e6ec93fb3e7048f4" -[[package]] -name = "libc-print" -version = "0.1.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4a660208db49e35faf57b37484350f1a61072f2a5becf0592af6015d9ddd4b0" -dependencies = [ - "libc", -] - [[package]] name = "libloading" version = "0.7.4" diff --git a/core/rs/core/Cargo.toml b/core/rs/core/Cargo.toml index 13ef9b804..c6464f5aa 100644 --- a/core/rs/core/Cargo.toml +++ b/core/rs/core/Cargo.toml @@ -15,7 +15,6 @@ sqlite_nostd = { path="../sqlite-rs-embedded/sqlite_nostd" } bytes = { version = "1.5", default-features = false } num-traits = { version = "0.2.17", default-features = false } num-derive = "0.4.1" -libc-print = "0.1.23" [dev-dependencies] diff --git a/core/rs/core/src/alter.rs b/core/rs/core/src/alter.rs index 398a88470..baf131208 100644 --- a/core/rs/core/src/alter.rs +++ b/core/rs/core/src/alter.rs @@ -33,12 +33,10 @@ unsafe fn compact_post_alter( errmsg: *mut *mut c_char, ) -> Result { let tbl_name_str = CStr::from_ptr(tbl_name).to_str()?; - libc_print::libc_println!("fill_db_version_if_needed"); fill_db_version_if_needed(db, ext_data).or_else(|msg| { errmsg.set(&msg); Err(ResultCode::ERROR) })?; - libc_print::libc_println!("DONE fill_db_version_if_needed"); let current_db_version = (*ext_data).dbVersion; // If primary key columns change (in the schema) @@ -65,7 +63,6 @@ unsafe fn compact_post_alter( drop(stmt); if pk_diff > 0 { - libc_print::libc_println!("dropping table, pk diff > 0!"); // drop the clock table so we can re-create it db.exec_safe(&format!( "DROP TABLE \"{table_name}__crsql_clock\"; @@ -85,9 +82,7 @@ unsafe fn compact_post_alter( tbl_name_val = crate::util::escape_ident_as_value(tbl_name_str), cl_sentinel = crate::c::DELETE_SENTINEL, ); - libc_print::libc_println!("compacting entries without a column... {}", sql); db.exec_safe(&sql)?; - libc_print::libc_println!("DONE compacting entries without a column..."); // Next delete entries that no longer have a row but keeping tombstones // TODO: if we move the sentinel metadata to the lookaside this becomes much simpler @@ -133,9 +128,7 @@ unsafe fn compact_post_alter( tbl_name = crate::util::escape_ident(tbl_name_str) ) ); - libc_print::libc_println!("deleting entries no longer have rows {}", sql); db.exec_safe(&sql)?; - libc_print::libc_println!("DONE deleteing entries no longer have rows"); // now delete pk lookasides that no longer map to anything in the clock tables let sql = format!( @@ -144,9 +137,7 @@ unsafe fn compact_post_alter( )", tbl_name = crate::util::escape_ident(tbl_name_str), ); - libc_print::libc_println!("deleting extra pk lookasides {}", sql); db.exec_safe(&sql)?; - libc_print::libc_println!("DONE deleting extra pk lookasides"); } let stmt = db.prepare_v2( diff --git a/core/rs/core/src/backfill.rs b/core/rs/core/src/backfill.rs index 7de39e3b0..63f326433 100644 --- a/core/rs/core/src/backfill.rs +++ b/core/rs/core/src/backfill.rs @@ -184,9 +184,7 @@ fn backfill_missing_columns( is_commit_alter: bool, ) -> Result { for non_pk_col in non_pk_cols { - libc_print::libc_println!("backfilling column {}", non_pk_col.name); - fill_column(db, table, pk_cols, &non_pk_col, is_commit_alter)?; - libc_print::libc_println!("DONE backfilling column {}", non_pk_col.name); + fill_column(db, table, pk_cols, non_pk_col, is_commit_alter)?; } Ok(ResultCode::OK) diff --git a/core/rs/core/src/create_crr.rs b/core/rs/core/src/create_crr.rs index 6a4bfe050..311a89367 100644 --- a/core/rs/core/src/create_crr.rs +++ b/core/rs/core/src/create_crr.rs @@ -36,7 +36,6 @@ pub fn create_crr( remove_crr_triggers_if_exist(db, table)?; create_triggers(db, &table_info, err)?; - libc_print::libc_println!("backfilling table"); backfill_table( db, table, @@ -45,7 +44,6 @@ pub fn create_crr( is_commit_alter, no_tx, )?; - libc_print::libc_println!("DONE backfilling table"); Ok(ResultCode::OK) } diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 646c58821..16a1662ed 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -662,8 +662,6 @@ unsafe extern "C" fn x_crsql_commit_alter( let non_destructive = if argc >= 3 { args[2].int() == 1 } else { false }; - libc_print::libc_println!("non-destructive? {}", non_destructive); - let ext_data = ctx.user_data() as *mut c::crsql_ExtData; let mut err_msg = null_mut(); let db = ctx.db_handle(); @@ -685,19 +683,13 @@ unsafe extern "C" fn x_crsql_commit_alter( } Err(rc) => rc as c_int, } - - // recreate_update_triggers(db, &table_info, &mut err_msg as *mut _) - - // crsql_ensure_table_infos_are_up_to_date(db, ext_data, &mut err_msg as *mut _) } else { - libc_print::libc_println!("compacting post alter"); let rc = crsql_compact_post_alter( db, table_name.as_ptr() as *const c_char, ext_data, &mut err_msg as *mut _, ); - libc_print::libc_println!("DONE compacting post alter"); if rc == ResultCode::OK as c_int { crsql_create_crr( @@ -713,29 +705,6 @@ unsafe extern "C" fn x_crsql_commit_alter( } }; - // let rc = if rc == ResultCode::OK as c_int { - // let table_infos = mem::ManuallyDrop::new(Box::from_raw( - // (*ext_data).tableInfos as *mut alloc::vec::Vec, - // )); - // crsql_create_crr( - // db, - // &table_infos, - // schema_name.as_ptr() as *const c_char, - // table_name.as_ptr() as *const c_char, - // 1, - // 0, - // &mut err_msg as *mut _, - // ) - // } else { - // rc - // }; - - // let rc = if non_destructive && rc == ResultCode::OK as c_int { - // crsql_ensure_table_infos_are_up_to_date(db, ext_data, &mut err_msg as *mut _) - // } else { - // rc - // }; - let rc = if rc == ResultCode::OK as c_int { db.exec_safe("RELEASE alter_crr") .unwrap_or(ResultCode::ERROR) as c_int diff --git a/core/rs/core/src/tableinfo.rs b/core/rs/core/src/tableinfo.rs index aa7352809..9adc4eb58 100644 --- a/core/rs/core/src/tableinfo.rs +++ b/core/rs/core/src/tableinfo.rs @@ -765,7 +765,6 @@ pub extern "C" fn crsql_ensure_table_infos_are_up_to_date( let mut table_infos = unsafe { Box::from_raw((*ext_data).tableInfos as *mut Vec) }; if schema_changed > 0 || table_infos.len() == 0 { - libc_print::libc_println!("schema changed, pulling tables"); match pull_all_table_infos(db, ext_data, err) { Ok(new_table_infos) => { *table_infos = new_table_infos; From e5946811e977e9c53ab3bea026e66b7c23f20f02 Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Thu, 28 Nov 2024 08:22:01 -0500 Subject: [PATCH 4/7] more cleanup of unused vars and importa --- core/rs/core/src/create_cl_set_vtab.rs | 4 ---- core/rs/core/src/lib.rs | 2 -- core/rs/core/src/tableinfo.rs | 5 ++--- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/core/rs/core/src/create_cl_set_vtab.rs b/core/rs/core/src/create_cl_set_vtab.rs index 3c07f4656..a4ba94868 100644 --- a/core/rs/core/src/create_cl_set_vtab.rs +++ b/core/rs/core/src/create_cl_set_vtab.rs @@ -1,16 +1,12 @@ extern crate alloc; use core::ffi::{c_char, c_int, c_void}; -use core::mem; use crate::alloc::borrow::ToOwned; -use crate::c::crsql_Changes_vtab; use crate::create_crr::create_crr; -use crate::tableinfo::TableInfo; use alloc::boxed::Box; use alloc::format; use alloc::string::String; -use alloc::vec::Vec; use sqlite::{sqlite3, Connection, CursorRef, StrRef, VTabArgs, VTabRef}; use sqlite_nostd as sqlite; use sqlite_nostd::ResultCode; diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 16a1662ed..19d3c436e 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -569,8 +569,6 @@ unsafe extern "C" fn x_crsql_as_crr( return; } - let ext_data = ctx.user_data() as *mut c::crsql_ExtData; - let rc = crsql_create_crr( db, schema_name.as_ptr() as *const c_char, diff --git a/core/rs/core/src/tableinfo.rs b/core/rs/core/src/tableinfo.rs index 9adc4eb58..102ca805c 100644 --- a/core/rs/core/src/tableinfo.rs +++ b/core/rs/core/src/tableinfo.rs @@ -855,15 +855,14 @@ pub fn pull_table_info( let mut cols: Vec = vec![]; while stmt.step()? == ResultCode::ROW { - let col = ColumnInfo { + cols.push(ColumnInfo { name: stmt.column_text(1)?.to_string(), cid: stmt.column_int(0), pk: stmt.column_int(2), curr_value_stmt: RefCell::new(None), merge_insert_stmt: RefCell::new(None), row_patch_data_stmt: RefCell::new(None), - }; - cols.push(col); + }); } if cols.len() != columns_len { From 3cc74784e39e01140e86631dc67d6b60f4c6e922 Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Thu, 28 Nov 2024 08:22:28 -0500 Subject: [PATCH 5/7] remove CommitAlter enum used during dev --- core/rs/core/src/lib.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 19d3c436e..8acf10078 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -892,14 +892,3 @@ pub extern "C" fn crsql_create_crr( _ => ResultCode::NOMEM as c_int, } } - -pub enum CommitAlter { - PossiblyDestructive, - NonDestructive, -} - -impl CommitAlter { - pub fn is_destructive(&self) -> bool { - matches!(self, CommitAlter::PossiblyDestructive) - } -} From b4e278d9074be3f5e92c2eee0a54865aeb22f5f1 Mon Sep 17 00:00:00 2001 From: Jerome Gravel-Niquet Date: Thu, 28 Nov 2024 08:24:25 -0500 Subject: [PATCH 6/7] somehow this cargo lock gets generated when building the sqlite3 target --- core/rs/bundle_static/Cargo.lock | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/core/rs/bundle_static/Cargo.lock b/core/rs/bundle_static/Cargo.lock index 909df5a63..a0858632d 100644 --- a/core/rs/bundle_static/Cargo.lock +++ b/core/rs/bundle_static/Cargo.lock @@ -108,7 +108,6 @@ name = "crsql_core" version = "0.1.0" dependencies = [ "bytes", - "libc-print", "num-derive", "num-traits", "sqlite_nostd", @@ -181,15 +180,6 @@ version = "0.2.148" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" -[[package]] -name = "libc-print" -version = "0.1.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4a660208db49e35faf57b37484350f1a61072f2a5becf0592af6015d9ddd4b0" -dependencies = [ - "libc", -] - [[package]] name = "libloading" version = "0.7.4" From 9cd3b4ca90f96cd90bccaf8112630bf90ca4e1c5 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 2 Dec 2024 18:18:43 +0100 Subject: [PATCH 7/7] create all triggers when commit_alter is run for non-destructive changes --- core/rs/core/src/lib.rs | 4 ++-- core/rs/core/src/triggers.rs | 27 --------------------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/core/rs/core/src/lib.rs b/core/rs/core/src/lib.rs index 8acf10078..f1224cb52 100644 --- a/core/rs/core/src/lib.rs +++ b/core/rs/core/src/lib.rs @@ -67,7 +67,7 @@ use sqlite_nostd as sqlite; use sqlite_nostd::{Connection, Context, Value}; use tableinfo::{crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info}; use teardown::*; -use triggers::recreate_update_triggers; +use triggers::create_triggers; pub extern "C" fn crsql_as_table( ctx: *mut sqlite::context, @@ -667,7 +667,7 @@ unsafe extern "C" fn x_crsql_commit_alter( let rc = if non_destructive { match pull_table_info(db, table_name, &mut err_msg as *mut _) { Ok(table_info) => { - match recreate_update_triggers(db, &table_info, &mut err_msg as *mut _) { + match create_triggers(db, &table_info, &mut err_msg) { Ok(ResultCode::OK) => { // need to ensure the right table infos in ext data crsql_ensure_table_infos_are_up_to_date( diff --git a/core/rs/core/src/triggers.rs b/core/rs/core/src/triggers.rs index 941fc754a..3a3cb0183 100644 --- a/core/rs/core/src/triggers.rs +++ b/core/rs/core/src/triggers.rs @@ -9,33 +9,6 @@ use sqlite_nostd as sqlite; use crate::tableinfo::TableInfo; -pub fn recreate_update_triggers( - db: *mut sqlite3, - table_info: &TableInfo, - err: *mut *mut c_char, -) -> Result { - db.exec_safe(&format!( - "DROP TRIGGER IF EXISTS \"{table}__crsql_utrig\"", - table = crate::util::escape_ident(&table_info.tbl_name) - ))?; - - // get all columns of table - // iterate pk cols - // drop triggers against those pk cols - let stmt = db.prepare_v2("SELECT name FROM pragma_table_info(?) WHERE pk > 0")?; - stmt.bind_text(1, &table_info.tbl_name, sqlite::Destructor::STATIC)?; - while stmt.step()? == ResultCode::ROW { - let col_name = stmt.column_text(0)?; - db.exec_safe(&format!( - "DROP TRIGGER IF EXISTS \"{tbl_name}_{col_name}__crsql_utrig\"", - tbl_name = crate::util::escape_ident(&table_info.tbl_name), - col_name = crate::util::escape_ident(col_name), - ))?; - } - - create_update_trigger(db, table_info, err) -} - pub fn create_triggers( db: *mut sqlite3, table_info: &TableInfo,