⚡ Bolt: reduce query execution allocations by removing redundant argument slice copies

[cungminh2710]

💡 What: Removed redundant slice copies in compiledQuery.literalArgs() and compiledQuery.bind() for queries that do not use named placeholders.
🎯 Why: Each query execution was performing an unnecessary heap allocation to copy the pre-calculated argument slice, even though the internal query runners do not modify it.
📊 Impact: Reduces heap allocations by 1 per query execution (e.g., from 42 to 41 for point lookups).
🔬 Measurement: Verified with go test -bench BenchmarkSQLiteSelectPointLookup/small -benchmem ./pkg/rain.

---

PR created automatically by Jules for task 13585493257413164491 started by @cungminh2710

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[greptile-apps]

Greptile Summary

This PR removes the append([]any(nil), q.args...) defensive copies inside literalArgs() and the !hasNames branch of bind(), returning the pre-calculated q.args slice directly to eliminate one heap allocation per query execution.

• The optimization is safe for all current call sites: ExecContext/QueryContext in database/sql create an internal []driver.NamedValue copy and do not write back to the []any arg slice; resolveCacheKey and buildQueryCacheKey only read from it.
• For prepared queries (PreparedInsertQuery, etc.), compiledQuery is compiled once and stored; bind() now returns a reference to the stored args backing array on every Exec/Scan call. The old comments explicitly documented the copy as protection against shared-state corruption; those guards are now gone without a replacement invariant documented in the code.

Confidence Score: 4/5

Safe to merge — no current caller modifies the returned slice, and database/sql never writes back to the variadic args array. The change achieves its allocation-reduction goal correctly for all existing paths.

Every call site that consumes the return value of literalArgs() or the !hasNames branch of bind() either passes the slice directly to ExecContext/QueryContext (which don't mutate it) or reads it for cache-key generation. For non-prepared queries the compiledQuery is ephemeral so there is no shared backing array at all. The sole concern is that prepared queries now expose the stored compiledQuery.args array on every execution call without any documented invariant preventing future code from mutating it in place, which would silently corrupt all subsequent executions of that prepared statement.

Only pkg/rain/query_compile.go is changed; the interaction with prepared_exec.go and prepared_select.go (which call bind() for long-lived prepared statements) deserves a second look to confirm no in-place mutation is introduced as those files evolve.

Important Files Changed

Filename	Overview
pkg/rain/query_compile.go	Removes defensive copies from literalArgs() and the !hasNames branch of bind(), returning q.args directly; safe for all current internal callers but silently changes the ownership contract for the public ToSQL() API and concurrent prepared-query execution paths.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant PreparedQuery
    participant compiledQuery
    participant db as database/sql

    Note over PreparedQuery,compiledQuery: Prepared at open time — compiledQuery stored in struct

    Caller->>PreparedQuery: "Exec(ctx, PreparedArgs{})"
    PreparedQuery->>compiledQuery: bind(args) [!hasNames path]
    Note over compiledQuery: Before PR: return append([]any(nil), q.args...)
    Note over compiledQuery: After PR: return q.args — shared backing array
    compiledQuery-->>PreparedQuery: bound ([]any)
    PreparedQuery->>db: stmt.ExecContext(ctx, bound...)
    Note over db: Creates internal []driver.NamedValue copy — does NOT modify bound
    db-->>PreparedQuery: sql.Result

    Note over Caller,db: Concurrent second call — same compiledQuery.args backing array returned
    Caller->>PreparedQuery: "Exec(ctx, PreparedArgs{})"
    PreparedQuery->>compiledQuery: bind(args)
    compiledQuery-->>PreparedQuery: same q.args reference
    PreparedQuery->>db: stmt.ExecContext(ctx, bound...)
    db-->>PreparedQuery: sql.Result

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
pkg/rain/query_compile.go:37-40
**Removed copy changes the slice-ownership contract for prepared queries**

For prepared types (`PreparedInsertQuery`, `PreparedUpdateQuery`, `PreparedDeleteQuery`, `PreparedSelectQuery`), the `compiledQuery` is compiled once and stored in the struct; every subsequent call to `Exec`/`Scan` calls `bind()` on that stored value. With this change, the `!hasNames` fast-path now hands back a reference to the same backing array (`p.compiled.args`) on every call. That's safe today because no current caller writes to the returned slice before passing it to `ExecContext`/`QueryContext`. However, the old comment explicitly named this as protection against "shared state footguns", and removing it silently drops the guard. If any future code path modifies `bound` in-place between `bind()` and the DB call (e.g., driver-specific wrapping, middleware, or retry logic), it will corrupt the stored `compiledQuery.args` for all subsequent calls — a data-race-free but logically corrupting mutation with no immediate panic. A `// NOTE: callers must not modify the returned slice` comment would preserve the intent without the allocation cost.

_{Reviews (1): Last reviewed commit: "⚡ Bolt: reduce query execution allocatio..." | Re-trigger Greptile}

[greptile-apps]

Removed copy changes the slice-ownership contract for prepared queries

For prepared types (PreparedInsertQuery, PreparedUpdateQuery, PreparedDeleteQuery, PreparedSelectQuery), the compiledQuery is compiled once and stored in the struct; every subsequent call to Exec/Scan calls bind() on that stored value. With this change, the !hasNames fast-path now hands back a reference to the same backing array (p.compiled.args) on every call. That's safe today because no current caller writes to the returned slice before passing it to ExecContext/QueryContext. However, the old comment explicitly named this as protection against "shared state footguns", and removing it silently drops the guard. If any future code path modifies bound in-place between bind() and the DB call (e.g., driver-specific wrapping, middleware, or retry logic), it will corrupt the stored compiledQuery.args for all subsequent calls — a data-race-free but logically corrupting mutation with no immediate panic. A // NOTE: callers must not modify the returned slice comment would preserve the intent without the allocation cost.

Prompt To Fix With AI

This is a comment left during a code review.
Path: pkg/rain/query_compile.go
Line: 37-40

Comment:
**Removed copy changes the slice-ownership contract for prepared queries**

For prepared types (`PreparedInsertQuery`, `PreparedUpdateQuery`, `PreparedDeleteQuery`, `PreparedSelectQuery`), the `compiledQuery` is compiled once and stored in the struct; every subsequent call to `Exec`/`Scan` calls `bind()` on that stored value. With this change, the `!hasNames` fast-path now hands back a reference to the same backing array (`p.compiled.args`) on every call. That's safe today because no current caller writes to the returned slice before passing it to `ExecContext`/`QueryContext`. However, the old comment explicitly named this as protection against "shared state footguns", and removing it silently drops the guard. If any future code path modifies `bound` in-place between `bind()` and the DB call (e.g., driver-specific wrapping, middleware, or retry logic), it will corrupt the stored `compiledQuery.args` for all subsequent calls — a data-race-free but logically corrupting mutation with no immediate panic. A `// NOTE: callers must not modify the returned slice` comment would preserve the intent without the allocation cost.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!