sqlite: avoid sqlite3_clear_bindings() before every run

Since SQLite 3.54, sqlite3_clear_bindings() invalidates the query plan
of the statement, so a statement was re-prepared on every run. Bind NULL
only to the parameters that were not bound in this run instead, and skip
clearing in the tag store, which always binds every parameter.

Refs: https://github.com/nodejs/node/issues/66800
Signed-off-by: dualfroz <me@dualfroz.com>
PR-URL: https://github.com/nodejs/node/pull/66983
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

src/node_sqlite.cc

@@ -3563,8 +3563,10 @@ inline int Statement::ResetStatement() {
 }
 
 bool Statement::BindParams(const FunctionCallbackInfo<Value>& args) {
-  int r = sqlite3_clear_bindings(statement_.get());
-  CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false);
+  int param_count = sqlite3_bind_parameter_count(statement_.get());
+  // Parameter indexing starts at one.
+  MaybeStackBuffer<bool, 32> bound(param_count + 1);
+  std::fill_n(bound.out(), param_count + 1, false);
 
   int anon_idx = 1;
   int anon_start = 0;
@@ -3580,8 +3582,6 @@ bool Statement::BindParams(const FunctionCallbackInfo<Value>& args) {
 
     if (allow_bare_named_params_ && !bare_named_params_.has_value()) {
       bare_named_params_.emplace();
-      int param_count = sqlite3_bind_parameter_count(statement_.get());
-      // Parameter indexing starts at one.
       for (int i = 1; i <= param_count; ++i) {
         const char* name = sqlite3_bind_parameter_name(statement_.get(), i);
         if (name == nullptr) {
@@ -3644,6 +3644,7 @@ bool Statement::BindParams(const FunctionCallbackInfo<Value>& args) {
       if (!BindValue(value, r)) {
         return false;
       }
+      bound[r] = true;
     }
     anon_start++;
   }
@@ -3659,10 +3660,20 @@ bool Statement::BindParams(const FunctionCallbackInfo<Value>& args) {
     if (!BindValue(args[i], anon_idx)) {
       return false;
     }
+    bound[anon_idx] = true;
 
     anon_idx++;
   }
 
+  // sqlite3_clear_bindings() would expire statements whose query plan depends
+  // on a bound value, so only reset the parameters that were not bound.
+  for (int i = 1; i <= param_count; ++i) {
+    if (!bound[i]) {
+      int r = sqlite3_bind_null(statement_.get(), i);
+      CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false);
+    }
+  }
+
   return true;
 }
 
@@ -4424,9 +4435,6 @@ bool SQLTagStore::ResetAndBindStatement(
   int r = stmt->ResetStatement();
   CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false);
 
-  r = sqlite3_clear_bindings(stmt->statement_.get());
-  CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, false);
-
   uint32_t n_params = args.Length() - 1;
   int param_count = sqlite3_bind_parameter_count(stmt->statement_.get());
   if (param_count != static_cast<int>(n_params)) {

test/parallel/test-sqlite-data-types.js

@@ -256,4 +256,32 @@ suite('data binding and mapping', () => {
       [{ __proto__: null, key: 1, val: 5 }, { __proto__: null, key: 2, val: null }],
     );
   });
+
+  test('omitted named parameters are unbound on each call', (t) => {
+    const db = new Database(':memory:');
+    t.after(() => { db.close(); });
+    const setup = db.exec(
+      'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER) STRICT;'
+    );
+    t.assert.strictEqual(setup, undefined);
+    const named = db.prepare('INSERT INTO data (key, val) VALUES ($k, $v)');
+    const mixed = db.prepare('INSERT INTO data (key, val) VALUES ($k, ?)');
+    named.run({ k: 1, v: 5 });
+    named.run({ k: 2 });
+    named.run({ k: 3, v: 6 });
+    named.run({ k: 4, $k: 4 });
+    mixed.run({ k: 5 }, 7);
+    mixed.run({ k: 6 });
+    t.assert.deepStrictEqual(
+      db.prepare('SELECT * FROM data ORDER BY key').all(),
+      [
+        { __proto__: null, key: 1, val: 5 },
+        { __proto__: null, key: 2, val: null },
+        { __proto__: null, key: 3, val: 6 },
+        { __proto__: null, key: 4, val: null },
+        { __proto__: null, key: 5, val: 7 },
+        { __proto__: null, key: 6, val: null },
+      ],
+    );
+  });
 });