Merge 'Fix: rolling back tx on Error should set autocommit to true' from Jussi Saurio

Rolling back a transaction on Error should result in
`connection.auto_commit` being set back to true.
Added a regression test for this where a UNIQUE constraint violation
rolls back the transaction and trying to COMMIT will fail.
Currently, our default conflict resolution strategy is ROLLBACK, which
ends the transaction. In SQLite, the default is ABORT, which rolls back
the current statement but allows the transaction to continue.
We should migrate to default ABORT once we support subtransactions.
Closes #3746

Reviewed-by: Preston Thorpe <preston@turso.tech>

Closes #3747
This commit is contained in:
Jussi Saurio
2025-10-16 16:55:54 +03:00
committed by GitHub
6 changed files with 43 additions and 1 deletions

2
Cargo.lock generated
View File

@@ -4828,6 +4828,8 @@ dependencies = [
"tempfile",
"thiserror 2.0.16",
"tokio",
"tracing",
"tracing-subscriber",
"turso_core",
]

View File

@@ -19,6 +19,8 @@ tracing_release = ["turso_core/tracing_release"]
[dependencies]
turso_core = { workspace = true, features = ["io_uring"] }
thiserror = { workspace = true }
tracing-subscriber.workspace = true
tracing.workspace = true
[dev-dependencies]
tempfile = { workspace = true }

View File

@@ -596,7 +596,11 @@ impl Statement {
pub async fn query_row(&mut self, params: impl IntoParams) -> Result<Row> {
let mut rows = self.query(params).await?;
rows.next().await?.ok_or(Error::QueryReturnedNoRows)
let first_row = rows.next().await?.ok_or(Error::QueryReturnedNoRows)?;
// Discard remaining rows so that the statement is executed to completion
// Otherwise Drop of the statement will cause transaction rollback
while rows.next().await?.is_some() {}
Ok(first_row)
}
}

View File

@@ -329,6 +329,7 @@ mod test {
#[tokio::test]
async fn test_drop() -> Result<()> {
let _ = tracing_subscriber::fmt::try_init();
let mut conn = checked_memory_handle().await?;
{
let tx = conn.transaction().await?;

View File

@@ -1002,6 +1002,7 @@ impl Program {
}
} else {
pager.rollback_tx(&self.connection);
self.connection.auto_commit.store(true, Ordering::SeqCst);
}
self.connection.set_tx_state(TransactionState::None);
}

View File

@@ -176,6 +176,38 @@ fn test_transaction_visibility() {
}
}
#[test]
/// Currently, our default conflict resolution strategy is ROLLBACK, which ends the transaction.
/// In SQLite, the default is ABORT, which rolls back the current statement but allows the transaction to continue.
/// We should migrate to default ABORT once we support subtransactions.
fn test_constraint_error_aborts_transaction() {
let tmp_db = TempDatabase::new("test_constraint_error_aborts_transaction.db", true);
let conn = tmp_db.connect_limbo();
// Create table succeeds
conn.execute("CREATE TABLE t (a INTEGER PRIMARY KEY)")
.unwrap();
// Begin succeeds
conn.execute("BEGIN").unwrap();
// First insert succeeds
conn.execute("INSERT INTO t VALUES (1),(2)").unwrap();
// Second insert fails due to UNIQUE constraint
let result = conn.execute("INSERT INTO t VALUES (2),(3)");
assert!(matches!(result, Err(LimboError::Constraint(_))));
// Commit fails because the transaction was aborted by the constraint error
let result = conn.execute("COMMIT");
assert!(matches!(result, Err(LimboError::TxError(_))));
// Make sure table is empty
let stmt = conn.query("SELECT COUNT(*) FROM t").unwrap().unwrap();
let row = helper_read_single_row(stmt);
assert_eq!(row, vec![Value::Integer(0)]);
}
#[test]
fn test_mvcc_transactions_autocommit() {
let tmp_db = TempDatabase::new_with_opts(