fix RETURNING, Statement::reset() and Program::abort() by jussisaurio · Pull Request #5478 · tursodatabase/turso

jussisaurio · 2026-02-20T08:42:08Z

Closes #4388
Closes #5490

Problem 1 - RETURNING implemented wrong

We were not executing RETURNING the same way as SQLite. Our way was:

INSERT ROW
RETURN ROW TO CALLER
INSERT ROW
RETURN ROW TO CALLER
...
COMMIT

SQlite does:

INSERT ROW
BUFFER RETURNING RESULT ROW TO TEMP TABLE
INSERT ROW
BUFFER RETURNING RESULT ROW TO TEMP TABLE
...
RETURN ROW FROM TEMP TABLE TO CALLER
RETURN ROW FROM TEMP TABLE TO CALLER
...
COMMIT

This is a problem because a naive implementation of our approach on main would result in this:

INSERT ROW
RETURN ROW TO CALLER
CALLER READS ROW AND DROPS/ABANDONS STATEMENT

Where not all INSERTs happened, so the statement would roll back. Think from user POV: you just executed INSERT INTO ... RETURNING ... and got a result row back, decided not to step through the rest and moved on. Now your tx is rolled back and the inserts didn't happen. Bad.

Our current workaround on main

On main, if there is a DML operation in progress, on Statement drop, we do a best-effort synchronous completion of the entire statement so that the DML effects of the statement persist.

This is not what SQLite does; instead SQLite immediately calls Halt and commits or rollbacks based on the rc (result code) on the VM.
@pereman2 thinks our run-statement-to-completion behavior is the cause of the bug Autocommit INSERT Returns Busy Error But Data Is Committed #5422 for MVCC.

Fix

Align RETURNING bytecode with SQLite; buffer to temp table and return from there.
Set a special has_returned_row flag indicating that we've emitted a ResultRow to caller.
In reset_internal if there's a DML statement (change_cnt_on) and we've emitted a row to caller, this signifies that all the DML is done and we now invoke Halt synchronously to commit the changes. In all other cases we roll back.
Consistent EXPLAIN naming for ephemeral tables

Problem 2 - Statement::reset() was infallible and not handling cleanup properly

Statement::reset() was infallible, although it can error in several points - errors were just logged. This can mask errors and also leave them unhandled, which can result in e.g. lock leaks.

Fix

Make reset() fallible; call reset_best_effort() from Statement::Drop - which does abort cleanup on panic unwind to ensure resources are released.

Problem 3 - Program::abort() could leave subjournal in use on error

This a problem because it can cause persistent Busy on future connections since they can't use the subjournal for statement subtransactions.

Fix

Capture error in Program::abort() and return it at the end, but release subjournal in all cases.

Tests

Some synchronous integration tests to verify behavior with RETURNING
Simulator tests using MemorySimIO for fault injection, to verify that abandoning statement after seeing StepResult::IO rolls back unless RETURNING scanback had already started.
Simulator tests to verify that reset() errors do not cause resource leaks (unfreed WAL locks / subjournal use) during panics or errors propagated from reset()

LeMikaelF · 2026-02-20T14:51:40Z

Small regression in this PR: changes() now counts rows in double:

CREATE TABLE t1(a INTEGER PRIMARY KEY, b TEXT);

-- Without RETURNING: correct
INSERT INTO t1 VALUES(1, 'a');
SELECT changes();  -- Returns: 1 ✓

-- With RETURNING: doubled
INSERT INTO t1 VALUES(10, 'r') RETURNING *;
SELECT changes();  -- Returns: 2  (expected: 1)

INSERT INTO t1 VALUES(11, 's'), (12, 't') RETURNING *;
SELECT changes();  -- Returns: 4  (expected: 2)

UPDATE t1 SET b = 'y' WHERE a >= 10 RETURNING *;
SELECT changes();  -- Returns: 6  (expected: 3)

DELETE FROM t1 WHERE a >= 10 RETURNING *;
SELECT changes();  -- Returns: 6  (expected: 3)

LeMikaelF · 2026-02-20T15:10:16Z

core/statement.rs

+                                break;
+                            }
+                        }
+                        Err(e) => {


does this mean that commits can fail silently?

Yeah - I made reset() fallible in 6847bf2 and added reset_best_effort() for Drop

Had to amend this a bit in 98db50b and c0ac8bc to ensure resources dont leak

LeMikaelF · 2026-02-20T15:20:47Z

testing/runner/tests/snapshot_tests/returning/snapshots/returning__upsert-returning.snap

+  41  Insert          0  24  25        0  intkey=r[25] data=r[24]
+  42  Goto            0  43   0        0
+  43  Goto            0  44   0        0
+  44  Rewind          0  50   0        0  Rewind  t2


bug in the comment here, this is the ephemeral table's cursor, but the comment says t2

fixed this and made ephemeral table namings consistent in EXPLAIN

…c IO

…g commit w/ async io

jussisaurio · 2026-02-23T09:42:02Z

Small regression in this PR: changes() now counts rows in double:

CREATE TABLE t1(a INTEGER PRIMARY KEY, b TEXT);

-- Without RETURNING: correct
INSERT INTO t1 VALUES(1, 'a');
SELECT changes();  -- Returns: 1 ✓

-- With RETURNING: doubled
INSERT INTO t1 VALUES(10, 'r') RETURNING *;
SELECT changes();  -- Returns: 2  (expected: 1)

INSERT INTO t1 VALUES(11, 's'), (12, 't') RETURNING *;
SELECT changes();  -- Returns: 4  (expected: 2)

UPDATE t1 SET b = 'y' WHERE a >= 10 RETURNING *;
SELECT changes();  -- Returns: 6  (expected: 3)

DELETE FROM t1 WHERE a >= 10 RETURNING *;
SELECT changes();  -- Returns: 6  (expected: 3)

fixed this - ephemeral inserts for returning were not marked with the ephemeral flag. added test.

Problem: Statement::reset_internal skipped abort/rollback cleanup while the thread was panicking. During unwind, Drop called reset_best_effort(), but reset_internal intentionally avoided cleanup paths, so connection transaction state could remain write-open after a panic. Why this is a problem: a panic in user code while a statement is running can leave transactional state and locks behind, blocking subsequent writes on the same connection and violating best-effort cleanup guarantees for Drop. Fix: execute reset cleanup paths regardless of std::thread::panicking(), and make reset_best_effort() panic-safe with catch_unwind so Drop never introduces a second panic. Errors are still logged, but cleanup is attempted during unwind.

Problem: reset/drop cleanup had two correctness holes. First, reset_internal aborted pending completion groups directly, which can panic (group callback invoked before all children finish). Second, abort(None, ...) could skip end_statement paths and leave subjournal ownership marked in-use, causing sticky Busy behavior for later statements. Why this is a problem: abandoning statements during IO or error handling can leave internal transactional resources in a bad state, trigger panics in cleanup, and block subsequent statements despite best-effort reset/drop semantics. Fix: drain pending statement IO via wait() during reset instead of force-aborting the group completion; always finalize subjournal ownership in Program::abort; clear uses_subjournal in ProgramState::reset as a defensive invariant; and add simulator regressions for cross-connection reset errors, panic-drop cleanup, and subjournal release on reset/drop.

LeMikaelF · 2026-02-23T14:51:32Z

core/vdbe/explain.rs

-    let get_table_or_index_name = |cursor_id: usize| {
+    let mut ephemeral_cursors = HashSet::new();
+    let mut changed = true;
+    while changed {


This can be simplified, if I understand correctly it's meant to prevent adding non-ephemeral cursors from OpenDup to ephemeral_cursors, but OpenDup can only ever be used with ephemeral tables, so line 38 can be done inconditionally, and the while loop can be removed.

LeMikaelF · 2026-02-23T16:18:16Z

core/statement.rs

+                                    e,
+                                    "Error committing during statement reset",
+                                );
+                                break;


nit: this could be simplified by breaking a Result<()> out of the loop (break e)

codspeed-hq · 2026-02-23T16:48:57Z

Merging this PR will improve performance by 12.68%

⚡ 3 improved benchmarks
✅ 276 untouched benchmarks
⏩ 105 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`cast_float_to_integer`	2.4 µs	2.1 µs	+12.68%
⚡	Simulation	`cast_text_to_integer`	2.8 µs	2.5 µs	+10.62%
⚡	Simulation	`rtrim_spaces`	1.1 µs	1 µs	+8.42%

_{Comparing fix-returning-again (d5a271f) with main (470dfb4)}

105 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions bot added simulator core translation/planning vdbe labels Feb 20, 2026

jussisaurio marked this pull request as ready for review February 20, 2026 08:46

jussisaurio requested review from penberg and pereman2 as code owners February 20, 2026 08:46

jussisaurio requested a review from sivukhin February 20, 2026 08:46

LeMikaelF reviewed Feb 20, 2026

View reviewed changes

amodhakal mentioned this pull request Feb 21, 2026

last_insert_rowid() returns 1 after INSERT with RETURNING #5490

Closed

jussisaurio added 6 commits February 23, 2026 10:17

translate: buffer RETURNING rows via ephemeral table and scan back

7df0c40

statement: reset unfinished writes correctly and clear pending io state

2a546a5

tests(integration): cover reset/drop semantics for RETURNING with syn…

ab1fc55

…c IO

tests(simulator): verify abandon during dml rollback vs post-returnin…

9b982ed

…g commit w/ async io

core: make statement reset fallible and keep drop best-effort

6847bf2

returning: exclude ephemeral buffer inserts from changes()

834c2cf

jussisaurio force-pushed the fix-returning-again branch from 6136e2d to 327dd5c Compare February 23, 2026 08:55

jussisaurio requested a review from PThorpe92 as a code owner February 23, 2026 08:55

github-actions bot added extensionlib Sqlite3 Perf/Benchmarks JS-Bindings Java-Bindings labels Feb 23, 2026

jussisaurio force-pushed the fix-returning-again branch from 327dd5c to 5b9eb68 Compare February 23, 2026 09:32

jussisaurio added 2 commits February 23, 2026 11:39

explain: disambiguate ephemeral cursors in bytecode comments

de61007

tests: add last_insert_rowid returning regression case

f23a822

jussisaurio force-pushed the fix-returning-again branch from 5b9eb68 to f23a822 Compare February 23, 2026 09:40

jussisaurio changed the title ~~fix: buffer RETURNING results into temp table and scan back & remove confusing statement reset behavior~~ fix RETURNING && stmt.reset() Feb 23, 2026

jussisaurio force-pushed the fix-returning-again branch from 92e7ac2 to c0ac8bc Compare February 23, 2026 11:55

jussisaurio changed the title ~~fix RETURNING && stmt.reset()~~ fix RETURNING, Statement::reset() and Program::abort() Feb 23, 2026

LeMikaelF reviewed Feb 23, 2026

View reviewed changes

LeMikaelF approved these changes Feb 23, 2026

View reviewed changes

simplify insn_to_row

d5a271f

jussisaurio merged commit 6d67663 into main Feb 23, 2026
88 checks passed

jussisaurio deleted the fix-returning-again branch February 23, 2026 17:45

jussisaurio mentioned this pull request Mar 3, 2026

Audit pager/wal etc properly for resources/locks that can leak on Drop #5023

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix RETURNING, Statement::reset() and Program::abort()#5478

fix RETURNING, Statement::reset() and Program::abort()#5478
jussisaurio merged 11 commits intomainfrom
fix-returning-again

jussisaurio commented Feb 20, 2026 •

edited

Loading

Uh oh!

LeMikaelF commented Feb 20, 2026

Uh oh!

LeMikaelF Feb 20, 2026

Uh oh!

jussisaurio Feb 23, 2026 •

edited

Loading

Uh oh!

jussisaurio Feb 23, 2026

Uh oh!

LeMikaelF Feb 20, 2026

Uh oh!

jussisaurio Feb 23, 2026

Uh oh!

jussisaurio commented Feb 23, 2026

Uh oh!

LeMikaelF Feb 23, 2026

Uh oh!

LeMikaelF Feb 23, 2026

Uh oh!

codspeed-hq bot commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jussisaurio commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem 1 - RETURNING implemented wrong

Our current workaround on main

Fix

Problem 2 - Statement::reset() was infallible and not handling cleanup properly

Fix

Problem 3 - Program::abort() could leave subjournal in use on error

Fix

Tests

Uh oh!

LeMikaelF commented Feb 20, 2026

Uh oh!

LeMikaelF Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jussisaurio Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jussisaurio Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

LeMikaelF Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jussisaurio Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

jussisaurio commented Feb 23, 2026

Uh oh!

LeMikaelF Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

LeMikaelF Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Feb 23, 2026

Merging this PR will improve performance by 12.68%

Performance Changes

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jussisaurio commented Feb 20, 2026 •

edited

Loading

jussisaurio Feb 23, 2026 •

edited

Loading