fix: number bare ? placeholders at parse time, in text order (found by SQE)

.openspec/adr/0044-bind-parameter-indices-assigned-at-parse-time.md

@@ -0,0 +1,87 @@
+# ADR-0044: Bind-parameter indices are assigned at parse time, in text order
+
+**Date:** 2026-09-10
+**Status:** Accepted
+
+## Context
+
+A bare `?` has no index written in the SQL; something has to assign one.
+SQLite does this while parsing, in `sqlite3ExprAssignVarNumber`: a bare `?`
+takes one more than the highest index used so far, and an explicit `?NNN`
+raises that high-water mark. The index is therefore a property of the SQL
+text.
+
+This crate assigned it during code generation instead, from a `next_param`
+counter on `RegAlloc` (`src/codegen.rs`), read by `compile_value`. That made
+the index a property of *compilation*, and two things about compilation broke
+it:
+
+- **Codegen does not visit expressions in text order.** `compile_update`
+  compiles the `WHERE` operand before the `SET` assignments, because the scan
+  has to be positioned before the row body is emitted. So
+  `UPDATE t SET v = ? WHERE k = ?` numbered the `WHERE` placeholder 1 and the
+  `SET` placeholder 2, and a caller binding in text order had its values
+  swapped.
+- **There is more than one `RegAlloc` per statement.** Eight sites call
+  `RegAlloc::new()`, each starting the counter at zero. A plan that compiles
+  part of a statement through a second allocator — the covering-index seek
+  path, for instance — restarted numbering mid-statement, collapsing two
+  distinct placeholders onto index 1.
+
+Both were silent. The swapped `UPDATE` matched no row and returned `Ok(0)`,
+which is also the rows-affected value an optimistic-concurrency check reads
+as "someone else won the race" — so a compare-and-swap built on it failed
+100% of the time while looking like ordinary contention. Both were reported
+by the first consumer to drive the embedding API with `?` rather than `?NNN`.
+
+Explicit `?NNN` was unaffected, which is why the existing parameter tests
+missed it: they were written with `?1`/`?2`, the form this repository's own
+code writes. `sqlx` — and most drivers — emit bare `?`.
+
+## Decision
+
+Assign the index in the parser, in text order, and carry it on the AST:
+`ParamKind::Anonymous(u32)`. `Parser` holds one `next_param` high-water mark,
+which is per-statement by construction because every parse entry point builds
+its own `Parser` for one statement's tokens. `?NNN` raises the mark; a
+following bare `?` continues past it. Codegen reads the index and no longer
+owns a counter, so `RegAlloc::anonymous_param` and
+`RegAlloc::numbered_param` are deleted along with the field they mutated.
+
+## Alternatives rejected
+
+**A numbering pass over the AST before codegen.** Leaves the AST shape and
+the parser untouched, and would fix both reported cases. Rejected because it
+needs a visitor that reaches every expression position in every statement
+type — `SET`, `WHERE`, `VALUES`, projections, `JOIN ON`, `HAVING`, `LIMIT`,
+subqueries, CTEs — and a position the visitor misses is not a compile error.
+It is this same bug, silently, in a shape nobody has tested yet. Assigning at
+the point of parse makes "was this numbered?" unrepresentable rather than
+merely tested.
+
+**Refusing bare `?` at prepare, the way named parameters are refused.**
+Provably safe and two lines. Rejected because bare `?` is what drivers
+generate: refusing it does not protect a consumer, it excludes them. Refusing
+named parameters is defensible because there is no index to bind them to;
+here the index exists and was simply computed in the wrong place.
+
+**Keeping the counter in codegen and making `compile_update` visit the `SET`
+list first.** Fixes the one reported statement and leaves the mechanism —
+plan-order-dependent numbering across eight allocators — in place for the
+next plan to trip over.
+
+## Consequences
+
+- `ParamKind::Anonymous` carries a `u32`. Five codegen match sites that
+  already accepted `Numbered(_)` alongside it needed `Anonymous(_)`; the
+  printer still renders `?`, because the source form is what it round-trips.
+- Numbering no longer depends on which plan the optimizer chose. This is the
+  substantive gain: it was previously possible for the same SQL to number its
+  parameters differently after an unrelated planner change, with no test
+  failing.
+- `Program::param_count()` (max `P1` over `Opcode::Variable`) becomes
+  trustworthy for bare `?`. It was reporting 1 for a two-placeholder
+  statement whenever the indices collapsed.
+- Parse-time assignment means a statement that never reaches codegen still
+  has its parameters numbered. That is what SQLite does and it is what
+  `sqlite3_bind_parameter_count` reports after `prepare`.

.openspec/adr/index.md

@@ -45,3 +45,4 @@ Specs record what the system must do; ADRs record **why it is shaped this way**
 | [0039](0039-value-payloads-are-arc-not-rc.md) | `Value`'s text and blob payloads are `Arc`, not `Rc` | 2026-09-04 |
 | [0040](0040-streaming-execution-with-batch-as-wrapper.md) | One streaming execution primitive, with the batch path as its wrapper | 2026-09-01 |
 | [0041](0041-embedding-api-owns-the-connection-driver-out-of-tree.md) | The embedding API owns the connection; the `sqlx` driver stays out of tree | 2026-08-28 |
+| [0044](0044-bind-parameter-indices-assigned-at-parse-time.md) | Bind-parameter indices are assigned at parse time, in text order | 2026-09-10 |

CHANGELOG.md

@@ -4,6 +4,24 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep
 
 **Versioning policy:** one minor version per completed plan phase — the version number tells the plan's story, sub-steps stay inside a phase. V1 (READ CORE) = 0.1.0 through 0.4.0. *(History note: internal iterations briefly numbered 0.4.0–0.6.0 were renumbered into the phase scheme on 14 Aug 2026, before any tag or publication of those versions existed.)*
 
+## [Unreleased]
+
+### Fixed
+
+- Bare `?` bind parameters were numbered during code generation rather than
+  at parse time, so their indices depended on the plan the optimizer chose
+  instead of on the order they appear in the SQL. Two consequences, both
+  silent: `UPDATE t SET v = ? WHERE k = ?` numbered its `WHERE` placeholder
+  before its `SET` one and bound the two values in the wrong order, matching
+  no row and reporting `Ok(0)` — which is also the value an
+  optimistic-concurrency check reads as a lost race; and a projection of
+  index columns only, on a table with a usable index, compiled its seek keys
+  through a second register allocator whose counter restarted, collapsing
+  two placeholders onto index 1 and reporting one parameter where there were
+  two. Indices are now assigned in the parser in text order, as SQLite does
+  it, so they no longer depend on the plan (ADR-0044). Explicit `?NNN` was
+  never affected.
+
 ## [0.18.10] - 2026-08-31
 
 ### Fixed

Cargo.toml

@@ -84,6 +84,10 @@ path = "tests/unit/pragma_synchronous_repl.rs"
 name = "unit_introspection_pragmas"
 path = "tests/unit/introspection_pragmas.rs"
 
+[[test]]
+name = "unit_param_numbering"
+path = "tests/unit/param_numbering_test.rs"
+
 [[test]]
 name = "unit_codegen"
 path = "tests/unit/codegen.rs"

src/codegen.rs

@@ -221,10 +221,6 @@ impl Emitter {
 #[derive(Debug)]
 pub(crate) struct RegAlloc {
     next: i32,
-    /// Next bind-parameter index to hand out for a bare `?`
-    /// (`ParamKind::Anonymous`) — 1-based, matching SQLite's
-    /// `sqlite3_bind_*` convention and `Opcode::Variable`'s `P1`.
-    next_param: u32,
     /// Next cursor number to hand out for a subquery's own scan (#238) —
     /// started well above every fixed cursor constant this compiler's
     /// other features use (`TABLE_CURSOR`/`SORT_CURSOR`/`PSEUDO_CURSOR`/
@@ -259,7 +255,6 @@ impl Default for RegAlloc {
     fn default() -> Self {
         RegAlloc {
             next: 0,
-            next_param: 0,
             next_cursor: 1000,
             materialized_ctes: Vec::new(),
         }
@@ -319,21 +314,6 @@ impl RegAlloc {
     pub(crate) fn peek(&self) -> i32 {
         self.next
     }
-
-    /// Assigns register-independent parameter index for a bare `?`,
-    /// incrementing past any `?NNN` index already claimed via
-    /// [`RegAlloc::numbered_param`].
-    pub(crate) fn anonymous_param(&mut self) -> u32 {
-        self.next_param = self.next_param.saturating_add(1);
-        self.next_param
-    }
-
-    /// Claims an explicit `?NNN` parameter index, advancing
-    /// `next_param` past it so a later bare `?` doesn't collide.
-    pub(crate) fn numbered_param(&mut self, n: u32) -> u32 {
-        self.next_param = self.next_param.max(n);
-        n
-    }
 }
 
 pub(crate) fn p4_coll_seq(

src/codegen/expr/value.rs

@@ -222,8 +222,7 @@ pub(crate) fn compile_value(
         ExprKind::Param(kind) => {
             let r = reg.alloc();
             let index = match kind {
-                ParamKind::Anonymous => Some(reg.anonymous_param()),
-                ParamKind::Numbered(n) => Some(reg.numbered_param(*n)),
+                ParamKind::Anonymous(n) | ParamKind::Numbered(n) => Some(*n),
                 ParamKind::Colon(_) | ParamKind::At(_) | ParamKind::Dollar(_) => None,
             };
             if let Some(index) = index {

src/codegen/select/aggregate.rs

@@ -94,7 +94,7 @@ where
             let is_supported_operand = matches!(
                 &operand.kind,
                 ExprKind::Literal(Literal::Integer(_))
-                    | ExprKind::Param(ParamKind::Anonymous | ParamKind::Numbered(_))
+                    | ExprKind::Param(ParamKind::Anonymous(_) | ParamKind::Numbered(_))
             );
             if !is_supported_operand {
                 return Ok(false);

src/codegen/select/join_order.rs

@@ -444,7 +444,7 @@ mod tests {
             Vec::<usize>::new()
         );
         let param = Expr {
-            kind: ExprKind::Param(ParamKind::Anonymous),
+            kind: ExprKind::Param(ParamKind::Anonymous(1)),
             span: span(),
         };
         assert_eq!(

src/codegen/select/limit_scan.rs

@@ -120,7 +120,7 @@ fn is_supported_seek_operand(expr: &Expr) -> bool {
     matches!(
         &expr.kind,
         ExprKind::Literal(Literal::Integer(_))
-            | ExprKind::Param(ParamKind::Anonymous | ParamKind::Numbered(_))
+            | ExprKind::Param(ParamKind::Anonymous(_) | ParamKind::Numbered(_))
     )
 }
 

src/codegen/select/range_scan.rs

@@ -86,7 +86,7 @@ pub(super) fn is_supported_operand(expr: &Expr) -> bool {
     matches!(
         &expr.kind,
         ExprKind::Literal(Literal::Integer(_) | Literal::Float(_) | Literal::Str(_))
-            | ExprKind::Param(ParamKind::Anonymous | ParamKind::Numbered(_))
+            | ExprKind::Param(ParamKind::Anonymous(_) | ParamKind::Numbered(_))
     )
 }
 

src/codegen/stmt/delete.rs

@@ -106,7 +106,7 @@ pub fn compile_delete_with_catalog(
             matches!(
                 &operand.kind,
                 ExprKind::Literal(Literal::Integer(_))
-                    | ExprKind::Param(ParamKind::Anonymous | ParamKind::Numbered(_))
+                    | ExprKind::Param(ParamKind::Anonymous(_) | ParamKind::Numbered(_))
             )
         });
 

src/codegen/stmt/update.rs

@@ -173,7 +173,7 @@ pub fn compile_update_with_catalog(
             matches!(
                 &operand.kind,
                 ExprKind::Literal(Literal::Integer(_))
-                    | ExprKind::Param(ParamKind::Anonymous | ParamKind::Numbered(_))
+                    | ExprKind::Param(ParamKind::Anonymous(_) | ParamKind::Numbered(_))
             )
         });
 

src/parser/ast.rs

@@ -506,8 +506,17 @@ pub enum Literal {
 /// A bind parameter's form.
 #[derive(Debug, Clone, PartialEq)]
 pub enum ParamKind {
-    /// Bare `?`.
-    Anonymous,
+    /// Bare `?`, carrying the 1-based index assigned at parse time.
+    ///
+    /// SQLite assigns this during parsing (`sqlite3ExprAssignVarNumber`),
+    /// in the order the placeholders appear in the SQL text: a bare `?`
+    /// takes one more than the highest index used so far, and a `?NNN`
+    /// raises that high-water mark. Carrying the index here rather than
+    /// deriving it in codegen is load-bearing — codegen visits
+    /// expressions in *plan* order, not text order, and uses more than
+    /// one register allocator per statement, so a codegen-time counter
+    /// numbers the same SQL differently depending on the plan chosen.
+    Anonymous(u32),
     /// `?NNN`.
     Numbered(u32),
     /// `:name`.

src/parser/grammar.rs

@@ -35,6 +35,11 @@ pub struct Parser {
     tokens: Vec<Token>,
     pos: usize,
     depth: usize,
+    /// Highest parameter index handed out so far, so a bare `?` can take
+    /// the next one in text order. Per-statement, which it is by
+    /// construction: every parse entry point builds its own `Parser` for
+    /// one statement's tokens.
+    next_param: u32,
 }
 
 /// Recursion-depth cap for `expr`/`not_expr`/`unary_expr`, so pathological
@@ -59,6 +64,7 @@ impl Parser {
             tokens,
             pos: 0,
             depth: 0,
+            next_param: 0,
         }
     }
 
@@ -2212,8 +2218,14 @@ impl Parser {
             TokenKind::Param(p) => {
                 self.advance();
                 let kind = match *p {
-                    Param::Anonymous => ParamKind::Anonymous,
-                    Param::Numbered(n) => ParamKind::Numbered(n),
+                    Param::Anonymous => {
+                        self.next_param = self.next_param.saturating_add(1);
+                        ParamKind::Anonymous(self.next_param)
+                    }
+                    Param::Numbered(n) => {
+                        self.next_param = self.next_param.max(n);
+                        ParamKind::Numbered(n)
+                    }
                     Param::Colon(s) => ParamKind::Colon(s),
                     Param::At(s) => ParamKind::At(s),
                     Param::Dollar(s) => ParamKind::Dollar(s),

src/parser/printer.rs

@@ -704,7 +704,7 @@ impl fmt::Display for Rollback {
 impl fmt::Display for ParamKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            ParamKind::Anonymous => write!(f, "?"),
+            ParamKind::Anonymous(_) => write!(f, "?"),
             ParamKind::Numbered(n) => write!(f, "?{n}"),
             ParamKind::Colon(s) => write!(f, ":{s}"),
             ParamKind::At(s) => write!(f, "@{s}"),