Repository: nodejs/node
Pull request: sqlite: reset statement immediately in run()

This commit updates `StatementSync.prototype.run()` to reset the prepared statement immediately after calling `sqlite3_step()` to return the correct change metadata.

Fixes: https://github.com/nodejs/node/issues/57344

This matches the better-sqlite3 implementation.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: src/node_sqlite.cc
patch: @@ -1866,13 +1866,9 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
-  auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
-  r = sqlite3_step(stmt->statement_);
-  if (r != SQLITE_ROW && r != SQLITE_DONE) {
-    THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_.get());
-    return;
-  }
-
+  sqlite3_step(stmt->statement_);
+  r = sqlite3_reset(stmt->statement_);
+  CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
   Local<Object> result = Object::New(env->isolate());
   sqlite3_int64 last_insert_rowid =
       sqlite3_last_insert_rowid(stmt->db_->Connection());

filename: test/parallel/test-sqlite-statement-sync.js
patch: @@ -170,6 +170,25 @@ suite('StatementSync.prototype.run()', () => {
       errstr: 'constraint failed',
     });
   });
+
+  test('returns correct metadata when using RETURNING', (t) => {
+    const db = new DatabaseSync(':memory:');
+    const setup = db.exec(
+      'CREATE TABLE data(key INTEGER PRIMARY KEY, val INTEGER NOT NULL) STRICT;'
+    );
+    t.assert.strictEqual(setup, undefined);
+    const sql = 'INSERT INTO data (key, val) VALUES ($k, $v) RETURNING key';
+    const stmt = db.prepare(sql);
+    t.assert.deepStrictEqual(
+      stmt.run({ k: 1, v: 10 }), { changes: 1, lastInsertRowid: 1 }
+    );
+    t.assert.deepStrictEqual(
+      stmt.run({ k: 2, v: 20 }), { changes: 1, lastInsertRowid: 2 }
+    );
+    t.assert.deepStrictEqual(
+      stmt.run({ k: 3, v: 30 }), { changes: 1, lastInsertRowid: 3 }
+    );
+  });
 });
 
 suite('StatementSync.prototype.sourceSQL', () => {