Repository: denoland/deno
Pull request: fix(ext/node): support numbered positional parameters in node:sqlite

SQLite numbered positional parameters (?1, ?2, ?NNN) were incorrectly
treated as named parameters and skipped during binding, causing a
"column index out of range" error. This fix updates the parameter
binding logic to recognize that parameters whose names start with '?'
are positional and should be bound in order, just like anonymous
parameters (?).

Fixes #31719


Changed files:
filename: ext/node/ops/sqlite/statement.rs
patch: @@ -543,20 +543,33 @@ impl StatementSync {
         anon_start += 1;
       }
 
-      let mut anon_idx = 1;
+      // SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
+      let sql_param_count = unsafe { ffi::sqlite3_bind_parameter_count(raw) };
+
+      let mut positional_idx = 1;
       for i in anon_start..params.length() {
-        // SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
-        while !unsafe { ffi::sqlite3_bind_parameter_name(raw, anon_idx) }
-          .is_null()
-        {
-          anon_idx += 1;
+        // Find the next positional parameter slot.
+        // Skip named parameters (:name, $name, @name) but include anonymous (?)
+        // and numbered (?NNN) parameters.
+        while positional_idx <= sql_param_count {
+          // SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
+          let name_ptr =
+            unsafe { ffi::sqlite3_bind_parameter_name(raw, positional_idx) };
+          if name_ptr.is_null()
+            // SAFETY: short-circuiting guarantees name_ptr is non-null here
+            || unsafe { *name_ptr as u8 == b'?' }
+          {
+            break;
+          }
+          // Named parameter (:name, $name, @name) - skip it
+          positional_idx += 1;
         }
 
         let value = params.get(i);
 
-        self.bind_value(scope, value, anon_idx)?;
+        self.bind_value(scope, value, positional_idx)?;
 
-        anon_idx += 1;
+        positional_idx += 1;
       }
     }
 

filename: tests/specs/run/sqlite_numbered_params/__test__.jsonc
patch: @@ -0,0 +1,8 @@
+{
+  "tests": {
+    "basic_numbered_params": {
+      "args": "run main.ts",
+      "output": "main.out"
+    }
+  }
+}

filename: tests/specs/run/sqlite_numbered_params/main.out
patch: @@ -0,0 +1,15 @@
+Test 1: Basic numbered parameters (?1, ?2)
+Insert changes: 1
+Inserted row: Alice alice@example.com
+
+Test 2: Parameter reuse (?1 appearing multiple times)
+Rows found: 3
+  id=1, parent_id=null
+  id=2, parent_id=1
+  id=3, parent_id=1
+
+Test 3: Parameters in different order (?2, ?1)
+Column a: second_arg
+Column b: first_arg
+
+All tests passed!

filename: tests/specs/run/sqlite_numbered_params/main.ts
patch: @@ -0,0 +1,64 @@
+import { DatabaseSync } from "node:sqlite";
+
+using db = new DatabaseSync(":memory:");
+
+// Test 1: Basic numbered parameters (?1, ?2)
+console.log("Test 1: Basic numbered parameters (?1, ?2)");
+db.exec(`
+  CREATE TABLE users (
+    id INTEGER PRIMARY KEY AUTOINCREMENT,
+    name TEXT NOT NULL,
+    email TEXT NOT NULL
+  )
+`);
+
+const insertStmt = db.prepare(
+  "INSERT INTO users (name, email) VALUES (?1, ?2)",
+);
+const result = insertStmt.run("Alice", "alice@example.com");
+console.log("Insert changes:", result.changes);
+
+const row = db.prepare("SELECT name, email FROM users WHERE id = 1").get() as {
+  name: string;
+  email: string;
+};
+console.log("Inserted row:", row.name, row.email);
+
+// Test 2: Parameter reuse (?1 appearing multiple times)
+console.log("\nTest 2: Parameter reuse (?1 appearing multiple times)");
+db.exec(`
+  CREATE TABLE nodes (
+    id INTEGER PRIMARY KEY,
+    parent_id INTEGER
+  )
+`);
+db.exec(
+  "INSERT INTO nodes (id, parent_id) VALUES (1, NULL), (2, 1), (3, 1), (4, 2)",
+);
+
+const reuseStmt = db.prepare(
+  "SELECT * FROM nodes WHERE id = ?1 OR parent_id = ?1 ORDER BY id",
+);
+const rows = reuseStmt.all(1) as { id: number; parent_id: number | null }[];
+console.log("Rows found:", rows.length);
+for (const r of rows) {
+  console.log(`  id=${r.id}, parent_id=${r.parent_id}`);
+}
+
+// Test 3: Parameters in different order (?2, ?1)
+console.log("\nTest 3: Parameters in different order (?2, ?1)");
+db.exec("CREATE TABLE test (a TEXT, b TEXT)");
+
+const orderStmt = db.prepare("INSERT INTO test (a, b) VALUES (?2, ?1)");
+orderStmt.run("first_arg", "second_arg");
+
+const testRow = db.prepare("SELECT a, b FROM test").get() as {
+  a: string;
+  b: string;
+};
+// first_arg binds to ?1, second_arg binds to ?2
+// SQL puts ?2 in column a, ?1 in column b
+console.log("Column a:", testRow.a);
+console.log("Column b:", testRow.b);
+
+console.log("\nAll tests passed!");

filename: tests/unit_node/sqlite_test.ts
patch: @@ -970,3 +970,68 @@ Deno.test("[node/sqlite] backup has correct name and length", () => {
   assertEquals(backup.name, "backup");
   assertEquals(backup.length, 2);
 });
+
+// https://github.com/denoland/deno/issues/31719 - numbered positional parameters
+Deno.test("[node/sqlite] numbered positional parameters (?1, ?2) should work", () => {
+  using db = new DatabaseSync(":memory:");
+  db.exec(`
+    CREATE TABLE users (
+      id INTEGER PRIMARY KEY AUTOINCREMENT,
+      name TEXT NOT NULL,
+      email TEXT NOT NULL
+    )
+  `);
+
+  // Test basic numbered parameters
+  const stmt = db.prepare("INSERT INTO users (name, email) VALUES (?1, ?2)");
+  const result = stmt.run("Alice", "alice@example.com");
+  assertEquals(result.changes, 1);
+
+  // Verify the data was inserted correctly
+  const row = db.prepare("SELECT name, email FROM users WHERE id = 1").get();
+  assertEquals(row, {
+    name: "Alice",
+    email: "alice@example.com",
+    __proto__: null,
+  });
+});
+
+// https://github.com/denoland/deno/issues/31719 - numbered positional parameters
+Deno.test("[node/sqlite] numbered parameters can be reused (?1 appearing multiple times)", () => {
+  using db = new DatabaseSync(":memory:");
+  db.exec(`
+    CREATE TABLE nodes (
+      id INTEGER PRIMARY KEY,
+      parent_id INTEGER
+    )
+  `);
+  db.exec(
+    "INSERT INTO nodes (id, parent_id) VALUES (1, NULL), (2, 1), (3, 1), (4, 2)",
+  );
+
+  // Use same parameter twice - important use case for SQLite
+  const stmt = db.prepare(
+    "SELECT * FROM nodes WHERE id = ?1 OR parent_id = ?1 ORDER BY id",
+  );
+  const rows = stmt.all(1);
+
+  assertEquals(rows.length, 3); // id=1, and two nodes with parent_id=1
+  assertEquals(rows[0], { id: 1, parent_id: null, __proto__: null });
+  assertEquals(rows[1], { id: 2, parent_id: 1, __proto__: null });
+  assertEquals(rows[2], { id: 3, parent_id: 1, __proto__: null });
+});
+
+// https://github.com/denoland/deno/issues/31719 - numbered positional parameters
+Deno.test("[node/sqlite] numbered parameters in different order (?2, ?1)", () => {
+  using db = new DatabaseSync(":memory:");
+  db.exec("CREATE TABLE test (a TEXT, b TEXT)");
+
+  // Parameters appear as ?2, ?1 in SQL but binding should still work positionally
+  const stmt = db.prepare("INSERT INTO test (a, b) VALUES (?2, ?1)");
+  stmt.run("first_arg", "second_arg");
+
+  const row = db.prepare("SELECT a, b FROM test").get();
+  // first_arg binds to ?1, second_arg binds to ?2
+  // SQL puts ?2 in column a, ?1 in column b
+  assertEquals(row, { a: "second_arg", b: "first_arg", __proto__: null });
+});