-
-
Notifications
You must be signed in to change notification settings - Fork 105
Open
Description
TL;DR
I'd like to replace a cleanup in automigrate_impl automigrate.rs on error: ROLLBACK (top level) -> ROLLBACK automigrate_tables + RELEASE automigrate_tables
The issue
Currently if an error is occurred, a top level ROLLBACK is performed:
// simplified:
fn automigrate_impl(
ctx: *mut sqlite::context,
args: &[*mut sqlite::value],
) -> Result<ResultCode, ResultCode> {
// ... setup
local_db.exec_safe("SAVEPOINT automigrate_tables;")?;
// ... do some op
if let Err(_) = migrate_result {
local_db.exec_safe("ROLLBACK")?;
}
// ... rest
}This is incompatible with the way automigration is run from the JS wrapper in crsqlite-wasm:
// crsqlite-wasm/src/DB.ts:DB (simplified)
async automigrateTo(
schemaName: string,
schemaContent: string
): Promise<"noop" | "apply" | "migrate"> {
// ... setup
await this.tx(async (tx) => {
await tx.exec(`SELECT crsql_automigrate(?, 'SELECT crsql_finalize();')`, [
schemaContent,
]);
});
// ... cleanup
}since "SELECT crsql_automigrate(...)" is called from within a .tx and .tx implementation introduces its own savepoints, expanding the execution to something like:
// ... setup
// from .tx impl - set SAVEPOINT
const id = 'crsql' + crypto.randomUUID().replaceAll("-", "")
await tx.exec("SAVEPOINT " + id)
try {
await tx.exec("SELECT crsql_automigrate(...)") // expanded cb (from automigtateTo)
} catch(e) {
await tx.exec("ROLLBACK TO " + id)
await tx.exec("RELEASE " + id)
throw e
}
// ... rest of the happy pathSo, in case of an error during crsql_automigrate, what happens is:
.txintroducesSAVEPOINT 1234567890crsql_automigrateintroducesSAVEPOINT automigrate_tablescrsql_automigrateerror out and does a top levelROLLBACK(clearing all other savepoints in global tx)tx.exec("SELECT crsql_automigrate(...)")errors outTXtries to clean up:ROLLBACK TO 1234567890+RELEASE 1234567890- this breaks as there's no savepoint
1234567890
Metadata
Metadata
Assignees
Labels
No labels