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

Fast commit alter when non-destructive change is made #445

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion core/rs/core/src/backfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ fn backfill_missing_columns(
is_commit_alter: bool,
) -> Result<ResultCode, ResultCode> {
for non_pk_col in non_pk_cols {
fill_column(db, table, pk_cols, &non_pk_col, is_commit_alter)?;
fill_column(db, table, pk_cols, non_pk_col, is_commit_alter)?;
}

Ok(ResultCode::OK)
Expand Down
1 change: 1 addition & 0 deletions core/rs/core/src/create_cl_set_vtab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ 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)
}

Expand Down
61 changes: 43 additions & 18 deletions core/rs/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,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::is_table_compatible;
use tableinfo::{crsql_ensure_table_infos_are_up_to_date, is_table_compatible, pull_table_info};
use teardown::*;
use triggers::create_triggers;

pub extern "C" fn crsql_as_table(
ctx: *mut sqlite::context,
Expand Down Expand Up @@ -567,6 +568,7 @@ unsafe extern "C" fn x_crsql_as_crr(
ctx.result_error("failed to start as_crr savepoint");
return;
}

let rc = crsql_create_crr(
db,
schema_name.as_ptr() as *const c_char,
Expand Down Expand Up @@ -650,34 +652,57 @@ 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 };

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 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(
db,
ext_data,
&mut err_msg as *mut _,
)
}
Ok(rc) | Err(rc) => rc as c_int,
}
}
Err(rc) => rc as c_int,
}
} else {
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
);

if rc == ResultCode::OK as c_int {
crsql_create_crr(
db,
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 {
db.exec_safe("RELEASE alter_crr")
.unwrap_or(ResultCode::ERROR) as c_int
Expand Down Expand Up @@ -859,11 +884,11 @@ 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) {
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
}
_ => ResultCode::NOMEM as c_int,
};
}
}
4 changes: 2 additions & 2 deletions core/rs/core/src/tableinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,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,
Expand All @@ -903,7 +903,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(
Expand Down
Loading