Skip to content

Prepared statement reuse rethrows the previous execution error #311

Description

@chrispader

Summary

The prepared-statement implementation in #294 reports a statement execution error twice: once from sqlite3_step(), then again on the next call when sqlite3_reset() returns the previous evaluation's error code.

Consequently, after a constraint failure, the next invocation can be rejected without executing its new parameters. A third invocation can then succeed because the prior sqlite3_reset() did reset the virtual machine despite returning the old error.

This was found while reviewing #294 at 4fe1d1bc4310570bb9c0a6652ee2b817c50948e0 against current main at ad8b835ba0f44a207649ecc2953820d39e4e8639 (v9.7.0). The public prepared-statement API exists only in the draft PR at the time of filing.

Code evidence

  • Statement execution throws immediately when sqlite3_step() returns an error: operations.cpp#L190-L203.
  • The next SQLitePreparedStatement::execute() calls sqlite3_reset() and treats any non-SQLITE_OK result as a new failure: operations.cpp#L304-L330.

SQLite documents that the return code from sqlite3_reset(S) indicates whether the previous evaluation completed successfully. If the most recent sqlite3_step(S) failed, reset returns that error code even though it resets the statement back to its initial state: https://www.sqlite.org/c3ref/reset.html

Reproduction against #294

const db = open({ name: 'prepared-reset.sqlite' })
db.execute('CREATE TABLE users (id INTEGER PRIMARY KEY)')

const insert = db.prepare('INSERT INTO users(id) VALUES (?)')

insert.execute([1])

// Correctly reports the UNIQUE/PRIMARY KEY constraint failure.
expect(() => insert.execute([1])).toThrow(/constraint/i)

// Expected: succeeds and inserts id=2.
// Current PR behavior: throws the previous constraint error without executing.
expect(insert.execute([2]).rowsAffected).toBe(1)

expect(
  db.execute<{ id: number }>('SELECT id FROM users ORDER BY id').rows._array,
).toEqual([{ id: 1 }, { id: 2 }])

insert.finalize()

Calling insert.execute([2]) a second time after the unexpected failure can succeed, demonstrating that the failed reset call already reset the VM and that the extra rejection belonged to the previous evaluation.

The same behavior applies to executeAsync() because both paths call the same native SQLitePreparedStatement::execute() implementation.

Impact

  • A long-lived prepared insert/update can appear poisoned for one additional invocation after any constraint, busy, schema, or other step error.
  • Applications may retry a write that never ran, producing confusing retry accounting and unnecessary user-visible failures.
  • ORMs and ingestion pipelines cannot safely reuse a prepared statement after handling a normal row-level constraint failure.

Proposed direction

Own reset/clear cleanup in the same execution attempt that stepped the statement. On both success and failure, leave the statement ready for the next call before returning or throwing.

The original step/execution error must remain the primary error for that invocation. If reset/cleanup reveals a distinct finalization error, preserve both without deferring the previous evaluation's status to the next call.

This lifecycle should be implemented together with the per-connection execution contract discussed in #303 and #304, so reset, rebinding, stepping, finalization, and close cannot race. Parameter binding and cleanup should continue to share the validation tracked in #309.

Acceptance criteria

  • A constraint failure is surfaced exactly once.
  • The very next call with valid parameters executes successfully.
  • Sync and async prepared execution have identical recovery behavior.
  • The statement is reset and bindings are cleared after successful execution and after execution failure.
  • The original execution error remains primary if cleanup also fails; a cleanup failure is not silently moved to the next invocation.
  • Harness tests cover successful reuse after SQLITE_CONSTRAINT for both execute() and executeAsync().
  • Tests verify the valid retry actually changed the database, rather than only checking that it did not throw.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions