From 56d570217675c1f729fb390b9a36bfc85bcd527e Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 16 Oct 2025 12:05:53 +0300 Subject: [PATCH 1/4] Fix: rolling back tx should set autocommit to true Rolling back a transaction 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. --- core/vdbe/mod.rs | 1 + .../query_processing/test_transactions.rs | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 584b62da4..962064fcd 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -992,6 +992,7 @@ impl Program { } } else { pager.rollback_tx(&self.connection); + self.connection.auto_commit.store(true, Ordering::SeqCst); } self.connection.set_tx_state(TransactionState::None); } diff --git a/tests/integration/query_processing/test_transactions.rs b/tests/integration/query_processing/test_transactions.rs index a96235153..36df15f9d 100644 --- a/tests/integration/query_processing/test_transactions.rs +++ b/tests/integration/query_processing/test_transactions.rs @@ -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( From 4de36d28e6b8c1a6d09932818956f1613fde3b49 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 16 Oct 2025 14:00:26 +0300 Subject: [PATCH 2/4] deps: add tracing to rust bindings --- Cargo.lock | 2 ++ bindings/rust/Cargo.toml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 9e0a26c13..c39f5ddaf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4828,6 +4828,8 @@ dependencies = [ "tempfile", "thiserror 2.0.16", "tokio", + "tracing", + "tracing-subscriber", "turso_core", ] diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml index d799b5320..42bce00cd 100644 --- a/bindings/rust/Cargo.toml +++ b/bindings/rust/Cargo.toml @@ -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 } From 6f1bda14380816c4e4936def1d4a4cff698809ee Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 16 Oct 2025 14:01:54 +0300 Subject: [PATCH 3/4] Instrument test_drop() with tracing --- bindings/rust/src/transaction.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bindings/rust/src/transaction.rs b/bindings/rust/src/transaction.rs index b68cc1fab..6da5c133d 100644 --- a/bindings/rust/src/transaction.rs +++ b/bindings/rust/src/transaction.rs @@ -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?; From 213af28cf3c24fb6d69be6b4f99e70facd546518 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 16 Oct 2025 14:02:07 +0300 Subject: [PATCH 4/4] rust bindings: make Statement::query:row() finish execution otherwise the statement will be considered to be in progress, and its Drop implementation will roll back the transaction it is in. --- bindings/rust/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bindings/rust/src/lib.rs b/bindings/rust/src/lib.rs index 5d87ad5f0..94a6556c9 100644 --- a/bindings/rust/src/lib.rs +++ b/bindings/rust/src/lib.rs @@ -596,7 +596,11 @@ impl Statement { pub async fn query_row(&mut self, params: impl IntoParams) -> Result { 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) } }