cr-sqlite icon indicating copy to clipboard operation
cr-sqlite copied to clipboard

Automgration: don't perform a top level ROLLBACK on error

Open ikusteu opened this issue 2 months ago • 0 comments

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 path

So, in case of an error during crsql_automigrate, what happens is:

  • .tx introduces SAVEPOINT 1234567890
  • crsql_automigrate introduces SAVEPOINT automigrate_tables
  • crsql_automigrate error out and does a top level ROLLBACK (clearing all other savepoints in global tx)
  • tx.exec("SELECT crsql_automigrate(...)") errors out
  • TX tries to clean up: ROLLBACK TO 1234567890 + RELEASE 1234567890
  • this breaks as there's no savepoint 1234567890

ikusteu avatar Oct 28 '25 08:10 ikusteu