put pk on tableinfo instead

Signed-off-by: Somtochi Onyekwere <[email protected]>

Author: somtochiama

core/rs/core/src/c.rs

@@ -74,7 +74,6 @@ pub struct crsql_ExtData {
     pub mergeEqualValues: ::core::ffi::c_int,
     pub timestamp: ::core::ffi::c_ulonglong,
     pub ordinalMap: *mut ::core::ffi::c_void,
-    pub clCache: *mut ::core::ffi::c_void,
 }
 
 #[repr(C)]
@@ -512,14 +511,4 @@ fn bindgen_test_layout_crsql_ExtData() {
             stringify!(ordinalMap)
         )
     );
-    assert_eq!(
-        unsafe { ::core::ptr::addr_of!((*ptr).clCache) as usize - ptr as usize },
-        168usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(crsql_ExtData),
-            "::",
-            stringify!(clCache)
-        )
-    );
 }

core/rs/core/src/changes_vtab.rs

@@ -587,18 +587,15 @@ pub extern "C" fn crsql_changes_rollback_to(vtab: *mut sqlite::vtab, _: c_int) -
         ))
     };
 
-    let mut cl_cache = unsafe {
+    let mut table_infos = unsafe {
         mem::ManuallyDrop::new(Box::from_raw(
-            (*(*tab).pExtData).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
+            (*(*tab).pExtData).tableInfos as *mut Vec<TableInfo>,
         ))
     };
-
-    for (_, map) in cl_cache.iter_mut() {
-        if !map.is_empty() {
-            map.clear(); 
-        }
+    for tbl_info in table_infos.iter_mut() {
+        tbl_info.clear_cl_cache();
     }
-    
+
     ordinals.clear();
     ResultCode::OK as c_int
 }

core/rs/core/src/changes_vtab_write.rs

@@ -1,8 +1,6 @@
 use alloc::boxed::Box;
-use alloc::collections::BTreeMap;
 use alloc::ffi::CString;
 use alloc::format;
-use alloc::string::String;
 use alloc::vec::Vec;
 use core::ffi::{c_char, c_int};
 use core::mem;
@@ -382,36 +380,45 @@ pub unsafe extern "C" fn crsql_merge_insert(
 
 fn get_local_cl(
     db: *mut sqlite::sqlite3,
-    tbl_info: &TableInfo,
+    tbl_info: &mut TableInfo,
     key: sqlite::int64,
 ) -> Result<sqlite::int64, ResultCode> {
-    let local_cl_stmt_ref = tbl_info.get_local_cl_stmt(db)?;
-    let local_cl_stmt = local_cl_stmt_ref.as_ref().ok_or(ResultCode::ERROR)?;
-
-    let rc = local_cl_stmt
-        .bind_int64(1, key)
-        .and_then(|_| local_cl_stmt.bind_int64(2, key));
-    if let Err(rc) = rc {
-        reset_cached_stmt(local_cl_stmt.stmt)?;
-        return Err(rc);
+    if let Some(cl) = tbl_info.get_cl(key) {
+        return Ok(*cl);
     }
 
-    let step_result = local_cl_stmt.step();
-    match step_result {
-        Ok(ResultCode::ROW) => {
-            let ret = local_cl_stmt.column_int64(0);
-            reset_cached_stmt(local_cl_stmt.stmt)?;
-            Ok(ret)
-        }
-        Ok(ResultCode::DONE) => {
+    let cl = {
+        let local_cl_stmt_ref = tbl_info.get_local_cl_stmt(db)?;
+        let local_cl_stmt = local_cl_stmt_ref.as_ref().ok_or(ResultCode::ERROR)?;
+
+        let rc = local_cl_stmt
+            .bind_int64(1, key)
+            .and_then(|_| local_cl_stmt.bind_int64(2, key));
+        if let Err(rc) = rc {
             reset_cached_stmt(local_cl_stmt.stmt)?;
-            Ok(0)
+            return Err(rc);
         }
-        Ok(rc) | Err(rc) => {
-            reset_cached_stmt(local_cl_stmt.stmt)?;
-            Err(rc)
+
+        let step_result = local_cl_stmt.step();
+        match step_result {
+            Ok(ResultCode::ROW) => {
+                let ret = local_cl_stmt.column_int64(0);
+                reset_cached_stmt(local_cl_stmt.stmt)?;
+                ret
+            }
+            Ok(ResultCode::DONE) => {
+                reset_cached_stmt(local_cl_stmt.stmt)?;
+                0
+            }
+            Ok(rc) | Err(rc) => {
+                reset_cached_stmt(local_cl_stmt.stmt)?;
+                return Err(rc);
+            }
         }
-    }
+    };
+
+    tbl_info.set_cl(key, cl);
+    Ok(cl)
 }
 
 unsafe fn merge_insert(
@@ -465,14 +472,10 @@ unsafe fn merge_insert(
 
     let insert_site_id = insert_site_id.blob();
 
-    let tbl_infos = mem::ManuallyDrop::new(Box::from_raw(
+    let mut tbl_infos = mem::ManuallyDrop::new(Box::from_raw(
         (*(*tab).pExtData).tableInfos as *mut Vec<TableInfo>,
     ));
 
-    let mut cl_cache = mem::ManuallyDrop::new(Box::from_raw(
-        (*(*tab).pExtData).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-    ));
-
     // TODO: will this work given `insert_tbl` is null termed?
     let tbl_info_index = tbl_infos.iter().position(|x| x.tbl_name == insert_tbl);
 
@@ -487,14 +490,14 @@ unsafe fn merge_insert(
     // TODO: technically safe since we checked `is_none` but this should be more idiomatic
     let tbl_info_index = tbl_info_index.unwrap();
 
-    let tbl_info = &tbl_infos[tbl_info_index];
+    let tbl_info = &mut tbl_infos[tbl_info_index];
     let unpacked_pks = unpack_columns(insert_pks.blob())?;
 
     // Get or create key as the first thing we do.
     // We'll need the key for all later operations.
     let key = tbl_info.get_or_create_key(db, &unpacked_pks)?;
 
-    let local_cl = get_pk_cl(db, &mut cl_cache, &tbl_info, key)?;
+    let local_cl = get_local_cl(db, tbl_info, key)?;
 
     // We can ignore all updates from older causal lengths.
     // They won't win at anything.
@@ -720,37 +723,9 @@ unsafe fn merge_insert(
 
         // a bigger cl always wins
         if insert_cl > local_cl {
-            match cl_cache.get_mut(&tbl_info.tbl_name) {
-                Some(x) => {
-                    x.insert(key, insert_cl);
-                }
-                None => {
-                    let mut new_map = BTreeMap::new();
-                    new_map.insert(key, insert_cl);
-                    cl_cache.insert(tbl_info.tbl_name.clone(), new_map);
-                }
-            }
+            tbl_info.set_cl(key, insert_cl);
         }
     }
 
     res
 }
-
-unsafe fn get_pk_cl(
-    db: *mut sqlite3,
-    cl_cache: &mut BTreeMap<String, BTreeMap<i64, i64>>,
-    tbl_info: &TableInfo,
-    key: sqlite::int64,
-) -> Result<sqlite::int64, ResultCode> {
-    match cl_cache.get(&tbl_info.tbl_name).and_then(|x| x.get(&key)) {
-        Some(cl) => Ok(*cl),
-        None => {
-            let cl = get_local_cl(db, tbl_info, key)?;
-            cl_cache
-                .entry(tbl_info.tbl_name.clone())
-                .or_default()
-                .insert(key, cl);
-            Ok(cl)
-        }
-    }
-}

core/rs/core/src/commit.rs

@@ -1,4 +1,4 @@
-use alloc::{boxed::Box, collections::BTreeMap, string::String, vec::Vec};
+use alloc::{boxed::Box, collections::BTreeMap, vec::Vec};
 use core::{
     ffi::{c_int, c_void},
     mem,
@@ -8,6 +8,7 @@ use core::{
 use sqlite_nostd::ResultCode;
 
 use crate::c::crsql_ExtData;
+use crate::tableinfo::TableInfo;
 
 #[no_mangle]
 pub unsafe extern "C" fn crsql_commit_hook(user_data: *mut c_void) -> c_int {
@@ -38,16 +39,12 @@ pub unsafe fn commit_or_rollback_reset(ext_data: *mut crsql_ExtData) {
         Box::from_raw((*ext_data).ordinalMap as *mut BTreeMap<Vec<u8>, i64>),
     );
 
-    let mut cl_cache = unsafe {
-        mem::ManuallyDrop::new(Box::from_raw(
-            (*ext_data).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-        ))
+    let mut table_infos = unsafe {
+        mem::ManuallyDrop::new(Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>))
     };
+    ordinals.clear();
 
-    for (_, map) in cl_cache.iter_mut() {
-        if !map.is_empty() {
-            map.clear();
-        }
+    for tbl_info in table_infos.iter_mut() {
+        tbl_info.clear_cl_cache();
     }
-    ordinals.clear();
 }

core/rs/core/src/db_version.rs

@@ -211,21 +211,6 @@ pub extern "C" fn crsql_drop_ordinal_map(ext_data: *mut crsql_ExtData) {
     }
 }
 
-#[no_mangle]
-pub extern "C" fn crsql_init_cl_cache(ext_data: *mut crsql_ExtData) {
-    let map: BTreeMap<String, BTreeMap<i64, i64>> = BTreeMap::new();
-    unsafe { (*ext_data).clCache = Box::into_raw(Box::new(map)) as *mut c_void }
-}
-
-#[no_mangle]
-pub extern "C" fn crsql_drop_cl_cache(ext_data: *mut crsql_ExtData) {
-    unsafe {
-        drop(Box::from_raw(
-            (*ext_data).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-        ));
-    }
-}
-
 pub fn insert_db_version(
     ext_data: *mut crsql_ExtData,
     insert_site_id: &[u8],

core/rs/core/src/lib.rs

@@ -49,7 +49,6 @@ mod unpack_columns_vtab;
 mod util;
 
 use alloc::format;
-use alloc::string::String;
 use alloc::string::ToString;
 use alloc::{borrow::Cow, boxed::Box, collections::BTreeMap, vec::Vec};
 use core::ffi::c_char;
@@ -74,7 +73,9 @@ 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};
+use tableinfo::{
+    crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info, TableInfo,
+};
 use teardown::*;
 use triggers::create_triggers;
 
@@ -1008,16 +1009,16 @@ unsafe extern "C" fn x_crsql_cache_pk_cl(
     let table_name = args[0].text();
     let pk_key = args[1].int64();
 
-    let cl_cache = mem::ManuallyDrop::new(Box::from_raw(
-        (*ext_data).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-    ));
+    let table_infos =
+        mem::ManuallyDrop::new(Box::from_raw((*ext_data).tableInfos as *mut Vec<TableInfo>));
+    let table_info = table_infos.iter().find(|t| t.tbl_name == table_name);
 
-    let table_map = cl_cache.get(table_name);
-    let cl = table_map
-        .and_then(|x| x.get(&pk_key))
-        .cloned()
-        .unwrap_or(-1);
-    sqlite::result_int64(ctx, cl);
+    if let Some(table_info) = table_info {
+        let cl = table_info.get_cl(pk_key).cloned().unwrap_or(-1);
+        sqlite::result_int64(ctx, cl);
+    } else {
+        ctx.result_error("table not found");
+    }
 }
 
 /**

core/rs/core/src/local_writes/after_delete.rs

@@ -1,8 +1,6 @@
 use alloc::string::String;
 use alloc::string::ToString;
-use alloc::{boxed::Box, collections::BTreeMap};
 use core::ffi::c_int;
-use core::mem;
 use sqlite::sqlite3;
 use sqlite::value;
 use sqlite::Context;
@@ -40,7 +38,7 @@ pub unsafe extern "C" fn x_crsql_after_delete(
 fn after_delete(
     db: *mut sqlite3,
     ext_data: *mut crsql_ExtData,
-    tbl_info: &TableInfo,
+    tbl_info: &mut TableInfo,
     pks_old: &[*mut value],
 ) -> Result<ResultCode, String> {
     let ts = unsafe { (*ext_data).timestamp.to_string() };
@@ -52,28 +50,22 @@ fn after_delete(
 
     let cl = mark_locally_deleted(db, tbl_info, key, db_version, seq, &ts)?;
 
-    // now actually delete the row metadata
-    let drop_clocks_stmt_ref = tbl_info
-        .get_merge_delete_drop_clocks_stmt(db)
-        .map_err(|_e| "failed to get mark_locally_deleted_stmt")?;
-    let drop_clocks_stmt = drop_clocks_stmt_ref
-        .as_ref()
-        .ok_or("Failed to deref sentinel stmt")?;
+    {
+        // now actually delete the row metadata
+        let drop_clocks_stmt_ref = tbl_info
+            .get_merge_delete_drop_clocks_stmt(db)
+            .map_err(|_e| "failed to get mark_locally_deleted_stmt")?;
+        let drop_clocks_stmt = drop_clocks_stmt_ref
+            .as_ref()
+            .ok_or("Failed to deref sentinel stmt")?;
 
-    drop_clocks_stmt
-        .bind_int64(1, key)
-        .map_err(|_e| "failed to bind pks to drop_clocks_stmt")?;
-    super::step_trigger_stmt(drop_clocks_stmt)?;
+        drop_clocks_stmt
+            .bind_int64(1, key)
+            .map_err(|_e| "failed to bind pks to drop_clocks_stmt")?;
+        super::step_trigger_stmt(drop_clocks_stmt)?;
+    }
 
-    let mut cl_cache = unsafe {
-        mem::ManuallyDrop::new(Box::from_raw(
-            (*ext_data).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-        ))
-    };
-    cl_cache
-        .entry(tbl_info.tbl_name.clone())
-        .or_default()
-        .insert(key, cl);
+    tbl_info.set_cl(key, cl);
 
     Ok(ResultCode::OK)
 }

core/rs/core/src/local_writes/after_insert.rs

@@ -1,9 +1,6 @@
-use alloc::boxed::Box;
-use alloc::collections::BTreeMap;
 use alloc::string::String;
 use alloc::string::ToString;
 use core::ffi::c_int;
-use core::mem;
 use sqlite::sqlite3;
 use sqlite::value;
 use sqlite::Context;
@@ -40,7 +37,7 @@ pub unsafe extern "C" fn x_crsql_after_insert(
 fn after_insert(
     db: *mut sqlite3,
     ext_data: *mut crsql_ExtData,
-    tbl_info: &TableInfo,
+    tbl_info: &mut TableInfo,
     pks_new: &[*mut value],
 ) -> Result<ResultCode, String> {
     let ts = unsafe { (*ext_data).timestamp.to_string() };
@@ -57,16 +54,7 @@ fn after_insert(
         // update the create record since it already exists.
         let seq = bump_seq(ext_data);
         let col_version = update_create_record(db, tbl_info, key_new, db_version, seq, &ts)?;
-
-        let mut cl_cache = unsafe {
-            mem::ManuallyDrop::new(Box::from_raw(
-                (*ext_data).clCache as *mut BTreeMap<String, BTreeMap<i64, i64>>,
-            ))
-        };
-        cl_cache
-            .entry(tbl_info.tbl_name.clone())
-            .or_default()
-            .insert(key_new, col_version);
+        tbl_info.set_cl(key_new, col_version);
     }
 
     super::mark_locally_inserted(db, ext_data, tbl_info, key_new, db_version, &ts)?;