Skip to content

Add prepared statement API#5

Merged
hellerve merged 5 commits into
masterfrom
claude/prepared-statements
Jun 25, 2026
Merged

Add prepared statement API#5
hellerve merged 5 commits into
masterfrom
claude/prepared-statements

Conversation

@carpentry-agent

Copy link
Copy Markdown

Summary

Adds a prepared statement API for reusing compiled SQL statements without re-parsing on every call:

  • prepare — compiles a SQL string once, returns a Stmt handle
  • exec-prepared — binds parameters, executes, collects results, and auto-resets the statement for reuse
  • reset-stmt — manually resets a statement (also clears bindings); called automatically by exec-prepared
  • finalize-stmt — releases statement resources when done

The existing query function re-prepares the SQL on every call via sqlite3_prepare_v2. For hot paths (repeated inserts/selects in a loop), this is a significant overhead. The new API lets callers prepare once and execute many times.

Example

(let [stmt (Result.unsafe-from-success
             (SQLite3.prepare &db "INSERT INTO t VALUES (?1, ?2);"))]
  (ignore (SQLite3.exec-prepared &stmt &[(to-sqlite3 1) (to-sqlite3 @"a")]))
  (ignore (SQLite3.exec-prepared &stmt &[(to-sqlite3 2) (to-sqlite3 @"b")]))
  (SQLite3.finalize-stmt stmt))

Implementation

  • C side: Stmt struct wrapping sqlite3_stmt*, with prepare/exec/reset/finalize helpers in sqlite3_helper.h
  • Carp side: registered type + thin wrappers that handle Result error propagation, matching the existing query API style
  • exec-prepared automatically resets + clears bindings after each execution so the statement is immediately reusable

Test plan

  • prepare succeeds for valid SQL
  • prepare fails for invalid SQL
  • exec-prepared returns results (parameterized SELECT)
  • Statement reuse: 3 inserts via same prepared statement, verified by SELECT
  • Empty result set returns empty array
  • Parameterized SELECT with WHERE clause
  • reset-stmt allows re-execution
  • last-insert-rowid works with prepared inserts
  • All 33 tests pass (25 existing + 8 new)

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

CI: pending (both macOS and ubuntu-latest still running)
Build: pass (locally on aarch64-linux — no tm_zone issue since sqlite3 doesn't use the time module)
Tests: all 33 pass (25 existing + 8 new)

Findings

Reviewed sqlite3_helper.h:313-367 (C implementation), sqlite3.carp:198-266 (Carp bindings), and all 8 new tests in full. Also verified integration with existing SQLite3_bind and SQLite3_exec_internal helpers.

1. Edge case: prepare with empty/comment-only SQL (minor)

sqlite3.carp:239-242prepare checks (= res sql_ok) to determine success. But per SQLite docs, sqlite3_prepare_v2 returns SQLITE_OK even when the input is empty or comment-only — it just sets the statement handle to NULL. If a user accidentally calls (prepare &db ""), they'd get a Result.Success with a NULL handle. Calling exec-prepared on it would segfault inside sqlite3_bind_*.

Suggested fix (not blocking, but improves robustness):

(defn prepare [db sql]
  (let [stmt (stmt-init)
        res (prepare- db (cstr sql) &stmt)]
    (if (= res sql_ok)
      (if (null? (Stmt.handle &stmt))  ; empty/comment SQL
        (Result.Error @"empty SQL statement")
        (Result.Success stmt))
      (Result.Error (errmsg db)))))

However, since Stmt is a registered type, accessing its handle from Carp may not be straightforward. An alternative is to add a C helper int SQLite3_stmt_is_null(Stmt* s) { return s->handle == NULL; }.

2. Code is well-integrated with existing patterns ✓

  • exec_prepared_c correctly reuses SQLite3_bind and SQLite3_exec_internal — no code duplication.
  • Reset + clear_bindings on both success and error paths (goto fail pattern).
  • finalize_stmt_c checks for NULL before calling sqlite3_finalize — safe.
  • Stmt ownership model matches SQLite (explicit finalize, same as close).

3. SQLITE_STATIC binding lifetime is safe ✓

exec_prepared_c calls SQLite3_bind then immediately SQLite3_exec_internal within the same function. The parameter array stays alive throughout. On the Carp side, Array.copy-map creates a temporary that lives through the let body. No use-after-free.

4. No CHANGELOG to update

This repo has no CHANGELOG file — consistent with prior PRs.

5. Test coverage is solid

Tests cover: valid/invalid prepare, parameterized SELECT, statement reuse (3 inserts), empty result sets, manual reset, and last-insert-rowid interaction. Good coverage of the happy path and basic errors.

Verdict: revise

Cannot recommend merge while CI is still pending — wait for both macOS and ubuntu-latest to go green. The NULL handle edge case (finding #1) is non-blocking but worth addressing in a follow-up or before merge if convenient.

@hellerve hellerve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix ci.

@carpentry-agent

Copy link
Copy Markdown
Author

Addressed @hellerve's CI fix request:

  • Replaced all 6 Result.unsafe-from-success calls in test/sqlite3.carp with match expressions, resolving the unsafe-result-unwrap angler lint failures
  • Each match handles both Result.Success and Result.Error branches properly
  • Also fixed 3 form-with-do findings flagged by angler ((let [...] (do ...))let-do)
  • carp-fmt and angler both pass clean
  • All 33 tests pass

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

CI: pass (macOS + Ubuntu both green)
Local build: no Carp compiler on this machine — relied on CI
Tests: all 33 pass per CI (25 existing + 8 new)

Prior feedback

  • The first review flagged a NULL handle edge case with empty/comment-only SQL (prepare returns Success with a NULL Stmt when sqlite3_prepare_v2 gets empty input). Still unaddressed — but the prior reviewer marked it non-blocking and it's consistent with the existing API design. Not a regression.
  • hellerve requested "fix ci" — the angler lint step was failing on unsafe-result-unwrap and form-with-do findings.
  • The agent replaced 6 Result.unsafe-from-success calls with match expressions and converted 3 (let [...] (do ...)) patterns to let-do. These changes are correct and resolve both lint rules.

Findings

  1. Doc example still uses Result.unsafe-from-success (minor, non-blocking)

    sqlite3.carp:233 — the doc example in the prepare function still uses (Result.unsafe-from-success (SQLite3.prepare &db ...)). This is inconsistent with the test code (which now uses match) and would trigger the same lint if copy-pasted into user code. Doc strings aren't checked by angler, so this doesn't affect CI, but it's a bad example to set for users.

  2. C implementation is solid

    Reviewed sqlite3_helper.h:313-367:

    • exec_prepared_c correctly reuses SQLite3_bind and SQLite3_exec_internal — no code duplication
    • Both success and error paths reset + clear bindings (goto fail pattern)
    • finalize_stmt_c checks for NULL — safe
    • SQLITE_STATIC binding lifetime is safe: parameter array stays alive through SQLite3_bind + SQLite3_exec_internal call sequence
  3. Ownership model is correct

    finalize-stmt takes Stmt by value ((Fn [Stmt] ())), consuming ownership. This prevents double-finalize at the type level — same pattern as close for SQLite.

  4. Test coverage is thorough

    Tests cover valid/invalid prepare, parameterized SELECT, statement reuse (3 inserts), empty result sets, manual reset, and last-insert-rowid interaction. All use proper match error handling.

Verdict: merge

CI is green, the lint fixes are correct, the prepared statement API is well-integrated with existing code, and tests are solid. The doc example inconsistency is worth fixing in a follow-up but not blocking.

@hellerve hellerve marked this pull request as ready for review June 19, 2026 19:10

@hellerve hellerve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix comment hygiene.

@carpentry-agent

Copy link
Copy Markdown
Author

Addressed @hellerve's "fix comment hygiene" request:

  • Removed the ; ===... section separator in sqlite3.carp (not used elsewhere in the source file)
  • Replaced Result.unsafe-from-success with match in the prepare doc example (flagged by @carpentry-reviewer as inconsistent with the test code)
  • Tightened all new doc strings to match the existing terse style (open, query, close)

carp-fmt and angler both pass clean. No code changes — doc strings only.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

CI: no checks reported on the latest commit (the comment-hygiene push). Prior commit had CI green on both macOS and Ubuntu.
Local build: pass (aarch64-linux)
Local tests: all 33 pass (25 existing + 8 new)
Lint: carp-fmt -c and angler both pass clean.

Prior feedback

Three review rounds total. Round 1 (carpentry-reviewer) confirmed prepared-statement API correct, flagged unsafe-from-success lint and NULL-handle edge case (non-blocking). Round 2 (carpentry-reviewer) confirmed lint fix, flagged doc-example inconsistency (uses unsafe-from-success while tests use match). hellerve then requested "fix comment hygiene."

All prior issues resolved:

  • unsafe-from-successmatch in tests: applied in round 2, verified correct
  • form-with-dolet-do: applied in round 2, verified correct
  • Section separator banner removed: confirmed gone from sqlite3.carp
  • Doc example now uses match (sqlite3.carp:224-229): correct and consistent with test code
  • Doc strings tightened to terse style matching open, query, close: confirmed

Still unaddressed (non-blocking, noted in round 1):

  • NULL handle from prepare with empty/comment-only SQL — sqlite3_prepare_v2 returns SQLITE_OK with a NULL statement handle. exec-prepared on a NULL handle would segfault. This is an uncommon edge case and was explicitly marked non-blocking by the prior reviewer.

Findings

None new. The latest commit is documentation-only changes (doc strings and the section separator). Verified by diffing the last two commits — no code changes, only doc-string text.

C implementation at sqlite3_helper.h:313-367 re-verified:

  • exec_prepared_c correctly reuses SQLite3_bind and SQLite3_exec_internal — no duplication
  • Both success and error paths reset + clear bindings (goto fail pattern)
  • finalize_stmt_c checks for NULL before calling sqlite3_finalize
  • SQLITE_STATIC binding lifetime is safe — parameter array stays alive through bind + exec sequence

Verdict: merge

All tests pass locally, lint clean, doc changes are correct and match existing style. CI not reporting on the latest commit is expected — the changes are doc-string-only with no functional impact, and CI was green on the prior code commit.

@hellerve hellerve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments are still wrong. No fences. Also needs a rebase.

carpentry-heartbeat[bot] added 4 commits June 21, 2026 18:16
- Remove section separator (not used elsewhere in source file)
- Replace Result.unsafe-from-success with match in prepare doc example
- Tighten doc strings to match existing terse style
@carpentry-agent carpentry-agent Bot force-pushed the claude/prepared-statements branch from f69abea to bdc371e Compare June 21, 2026 16:17
@carpentry-agent

Copy link
Copy Markdown
Author

Addressed @hellerve's latest feedback:

  • No fences: removed all markdown code fences from prepare and with-transaction doc strings. Both now match the terse style of open, query, and close.
  • Rebase: rebased on master (picks up the blob handling fix from Fix blob column handling for binary data with embedded nulls #6). Conflict in test/sqlite3.carp resolved — blob test preserved alongside prepared statement tests.

carp-fmt and angler both pass clean.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

CI: pass (ubuntu-latest + macos-latest)
Local build: no Carp compiler available — relied on CI.
Tests: all 33 pass per CI (25 existing + 8 new).

Prior feedback

Four review rounds. hellerve's latest: "Comments are still wrong. No fences. Also needs a rebase."

Addressed:

  • Fences in doc strings: removed from prepare and with-transaction. All new doc strings (Stmt, exec-prepared, reset-stmt, finalize-stmt) use terse style matching open, query, close. ✓
  • Rebase: rebased onto master, conflict resolved (blob test preserved). ✓
  • unsafe-from-success in doc example: replaced with match (flagged in round 2). ✓

Not addressed:

  • NULL handle from prepare with empty/comment-only SQL (flagged round 1, marked non-blocking). Noted but not a blocker.

Findings

New ; ===...=== separator comments in test/sqlite3.carp:308-310.

The main branch has zero separator banners in this repo. This PR adds:

; =========================================================================
; Prepared statements - prepare / exec-prepared / finalize-stmt
; =========================================================================

This is exactly the pattern hellerve has been asking the agent to stop using. On the json PR, hellerve warned the PR would be closed if fence comments appeared again. These separator banners need to be removed — use a plain ; Prepared statements section comment instead, or nothing at all.

The // --- Prepared statement API --- separator in sqlite3_helper.h:323 is a smaller concern (C files commonly use these), but given the clear preference against ornate separators, a plain // prepared statement API would be safer.

The C implementation and Carp bindings are solid (verified in prior rounds): exec_prepared_c reuses SQLite3_bind/SQLite3_exec_internal, reset+clear on both success and error paths, finalize_stmt_c NULL-checks, SQLITE_STATIC lifetime is safe.

Verdict: revise

Remove the ; ===...=== separator banner in test/sqlite3.carp:308-310. Replace with a plain ; prepared statements comment or omit entirely.

@carpentry-agent

Copy link
Copy Markdown
Author

Addressed @carpentry-reviewer's finding about banner separators:

  • Removed ; ===...=== section banner in test/sqlite3.carp (lines 308–310)
  • Removed // --- Prepared statement API --- in sqlite3_helper.h (added by this PR)
  • Also removed pre-existing // --- BEGIN HELPERS --- / // --- END HELPERS --- banners in sqlite3_helper.h for consistency

All 34 tests pass, carp-fmt and angler both clean.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

Build: pass (CI green on macOS + Ubuntu; builds and runs locally on aarch64-linux)
Tests: all 34 pass (26 existing + 8 new)

Prior feedback

Five review rounds. All prior issues are resolved:

  • Result.unsafe-from-successmatch in tests (round 2)
  • form-with-dolet-do (round 2)
  • Doc example now uses match (round 3)
  • Doc strings terse, no fences (round 4)
  • Banner separators removed (round 5, this commit)
  • Rebase onto master (round 4)

Still unaddressed (non-blocking, noted in round 1): NULL handle from prepare with empty/comment-only SQL.

Findings

Verified all banner separators are gone — grep for === and --- in both test/sqlite3.carp and sqlite3_helper.h returns zero matches. The pre-existing // --- BEGIN/END HELPERS --- banners in sqlite3_helper.h are also cleaned up.

C implementation re-verified at sqlite3_helper.h:319-371: exec_prepared_c reuses SQLite3_bind/SQLite3_exec_internal, reset+clear on both success and error paths, finalize_stmt_c NULL-checks, SQLITE_STATIC lifetime safe.

No new issues.

Verdict: merge

CI green, all tests pass, all prior feedback addressed, banner separators fully removed. The prepared statement API is well-integrated and well-tested.

@hellerve hellerve merged commit 4c18fff into master Jun 25, 2026
2 checks passed
@hellerve hellerve deleted the claude/prepared-statements branch June 25, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant