sqlite: bind undefined to NULL

Omitting a named parameter binds NULL, but passing `undefined` for that
same parameter threw ERR_INVALID_ARG_TYPE. Bind `undefined` to NULL so
the two forms agree. This matches the conversion already applied to a
user-defined function's `undefined` return value, as well as SQLite's
own WASM oo1 API.

Fixes: #61824
Refs: #61472
Refs: #62008
Co-authored-by: mike-git374 <217764531+mike-git374@users.noreply.github.com>
Assisted-by: claude:opus-5
Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>

Author: TrevorBurnham

doc/api/sqlite.md

@@ -101,7 +101,7 @@ exception.
 
 | Storage class | JavaScript to SQLite                                            | SQLite to JavaScript                  |
 | ------------- | --------------------------------------------------------------- | ------------------------------------- |
-| `NULL`        | {null}                                                          | {null}                                |
+| `NULL`        | {null} or {undefined}                                           | {null}                                |
 | `INTEGER`     | {number}, {bigint}, or {boolean}                                | {number} or {bigint} _(configurable)_ |
 | `REAL`        | {number}                                                        | {number}                              |
 | `TEXT`        | {string}                                                        | {string}                              |
@@ -113,6 +113,10 @@ values (`1n` and `0n`) when reading BigInts is enabled. Writing a {bigint} that
 does not fit in a signed 64-bit integer throws an `ERR_INVALID_ARG_VALUE`
 error.
 
+`undefined` is written as `NULL`, so passing it explicitly is equivalent to
+omitting a named parameter. `NULL` always reads back as {null}, never
+`undefined`.
+
 APIs that read values from SQLite have a configuration option that determines
 whether `INTEGER` values are converted to `number` or `bigint` in JavaScript,
 such as the `readBigInts` option for statements and the `useBigIntArguments`
@@ -1089,6 +1093,11 @@ Binding a key that does not name a parameter of the statement throws an
 `ERR_INVALID_STATE` error unless unknown named parameters are ignored. See
 [`statement.setAllowUnknownNamedParameters()`][].
 
+Parameters that are never bound are `NULL`, and binding `undefined` has the same
+effect, so `{ $a: undefined }` and `{}` are equivalent. Because `undefined` is
+not an object, passing it in place of `namedParameters` binds it as an anonymous
+parameter instead.
+
 See [Type conversion between JavaScript and SQLite][] for the values that can be
 bound. Binding any other value throws an `ERR_INVALID_ARG_TYPE` error.
 
@@ -1097,6 +1106,9 @@ bound. Binding any other value throws an `ERR_INVALID_ARG_TYPE` error.
 <!-- YAML
 added: v22.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1112,7 +1124,8 @@ changes:
 
 * `namedParameters` {Object} An optional object used to bind named parameters.
   The keys of this object are used to configure the mapping.
-* `...anonymousParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...anonymousParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Zero or more values to bind to anonymous parameters.
 * Returns: {Array} An array of objects. Each object corresponds to a row
   returned by executing the prepared statement. The keys and values of each
@@ -1185,6 +1198,9 @@ execution of this prepared statement. This property is a wrapper around
 <!-- YAML
 added: v22.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1200,7 +1216,8 @@ changes:
 
 * `namedParameters` {Object} An optional object used to bind named parameters.
   The keys of this object are used to configure the mapping.
-* `...anonymousParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...anonymousParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Zero or more values to bind to anonymous parameters.
 * Returns: {Object|undefined} An object corresponding to the first row returned
   by executing the prepared statement. The keys and values of the object
@@ -1220,6 +1237,9 @@ added:
   - v23.4.0
   - v22.13.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1235,7 +1255,8 @@ changes:
 
 * `namedParameters` {Object} An optional object used to bind named parameters.
   The keys of this object are used to configure the mapping.
-* `...anonymousParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...anonymousParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Zero or more values to bind to anonymous parameters.
 * Returns: {Iterator} An iterable iterator of objects. Each object corresponds to a row
   returned by executing the prepared statement. The keys and values of each
@@ -1264,6 +1285,9 @@ executions of the same prepared statement.
 <!-- YAML
 added: v22.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1279,7 +1303,8 @@ changes:
 
 * `namedParameters` {Object} An optional object used to bind named parameters.
   The keys of this object are used to configure the mapping.
-* `...anonymousParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...anonymousParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Zero or more values to bind to anonymous parameters.
 * Returns: {Object}
   * `changes` {number|bigint} The number of rows modified, inserted, or deleted
@@ -1448,6 +1473,9 @@ class execute synchronously.
 <!-- YAML
 added: v24.9.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1458,7 +1486,8 @@ changes:
 
 * `stringElements` {string\[]} Template literal elements containing the SQL
   query.
-* `...boundParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...boundParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Parameter values to be bound to placeholders in the template string.
 * Returns: {Array} An array of objects representing the rows returned by the query.
 
@@ -1473,6 +1502,9 @@ called directly.
 <!-- YAML
 added: v24.9.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1483,7 +1515,8 @@ changes:
 
 * `stringElements` {string\[]} Template literal elements containing the SQL
   query.
-* `...boundParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...boundParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Parameter values to be bound to placeholders in the template string.
 * Returns: {Object | undefined} An object representing the first row returned by
   the query, or `undefined` if no rows are returned.
@@ -1498,6 +1531,9 @@ called directly.
 <!-- YAML
 added: v24.9.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1508,7 +1544,8 @@ changes:
 
 * `stringElements` {string\[]} Template literal elements containing the SQL
   query.
-* `...boundParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...boundParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Parameter values to be bound to placeholders in the template string.
 * Returns: {Iterator} An iterator that yields objects representing the rows returned by the query.
 
@@ -1522,6 +1559,9 @@ called directly.
 <!-- YAML
 added: v24.9.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65709
+    description: Bind `undefined` to `NULL`.
   - version: v26.8.0
     pr-url: https://github.com/nodejs/node/pull/62001
     description: Add support for boolean values in bound parameters.
@@ -1532,7 +1572,8 @@ changes:
 
 * `stringElements` {string\[]} Template literal elements containing the SQL
   query.
-* `...boundParameters` {null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
+* `...boundParameters`
+  {undefined|null|number|bigint|boolean|string|Buffer|TypedArray|DataView|ArrayBuffer|SharedArrayBuffer}
   Parameter values to be bound to placeholders in the template string.
 * Returns: {Object} An object containing information about the execution, including `changes` and `lastInsertRowid`.
 

src/node_sqlite.cc

@@ -3101,7 +3101,8 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
   // Dates could be supported by converting them to numbers. However, there
   // would not be a good way to read the values back from SQLite with the
   // original type. JS Boolean binds to 1 and 0 because SQLite maps true and
-  // false keywords to 1 and 0.
+  // false keywords to 1 and 0. JS undefined binds to NULL so that passing it
+  // explicitly matches omitting the parameter altogether.
   Isolate* isolate = env()->isolate();
   int r;
   if (value->IsNumber()) {
@@ -3125,7 +3126,7 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
                               SQLITE_TRANSIENT,
                               SQLITE_UTF8);
     }
-  } else if (value->IsNull()) {
+  } else if (value->IsNullOrUndefined()) {
     r = sqlite3_bind_null(statement_.get(), index);
   } else if (value->IsArrayBufferView() || value->IsArrayBuffer() ||
              value->IsSharedArrayBuffer()) {

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

@@ -80,6 +80,70 @@ suite('data binding and mapping', () => {
       query.get(5),
       { __proto__: null, key: 5, int: 1, double: 0, text: '1', buf: null }
     );
+
+    t.assert.deepStrictEqual(
+      stmt.run(6, undefined, undefined, undefined, undefined),
+      { changes: 1, lastInsertRowid: 6 }
+    );
+    t.assert.deepStrictEqual(
+      query.get(6),
+      { __proto__: null, key: 6, int: null, double: null, text: null, buf: null }
+    );
+  });
+
+  test('undefined is bound as NULL', (t) => {
+    const db = new DatabaseSync(':memory:');
+    t.after(() => { db.close(); });
+    const setup = db.exec(
+      'CREATE TABLE types(key INTEGER PRIMARY KEY, val INTEGER) STRICT;'
+    );
+    t.assert.strictEqual(setup, undefined);
+    const stmt = db.prepare('INSERT INTO types (key, val) VALUES ($k, $v)');
+    const query = db.prepare('SELECT * FROM types WHERE key = ?');
+
+    // An explicit `undefined` and an omitted named parameter are equivalent.
+    t.assert.deepStrictEqual(
+      stmt.run({ k: 1, v: undefined }),
+      { changes: 1, lastInsertRowid: 1 }
+    );
+    t.assert.deepStrictEqual(
+      stmt.run({ k: 2 }),
+      { changes: 1, lastInsertRowid: 2 }
+    );
+    t.assert.deepStrictEqual(
+      query.get(1),
+      { __proto__: null, key: 1, val: null }
+    );
+    t.assert.deepStrictEqual(
+      query.get(2),
+      { __proto__: null, key: 2, val: null }
+    );
+
+    t.assert.deepStrictEqual(
+      db.prepare('SELECT ? AS a').get(undefined),
+      { __proto__: null, a: null }
+    );
+  });
+
+  test('undefined is not treated as the named parameters argument', (t) => {
+    const db = new DatabaseSync(':memory:');
+    t.after(() => { db.close(); });
+
+    // `undefined` is not an object, so it is bound as an anonymous parameter
+    // rather than being treated as the named parameters argument.
+    t.assert.throws(() => {
+      db.prepare('SELECT $k AS a').get(undefined);
+    }, {
+      code: 'ERR_SQLITE_ERROR',
+      message: /column index out of range/,
+    });
+
+    t.assert.throws(() => {
+      db.prepare('SELECT ? AS a').get(1, undefined);
+    }, {
+      code: 'ERR_SQLITE_ERROR',
+      message: /column index out of range/,
+    });
   });
 
   test('large strings are bound correctly', (t) => {
@@ -126,7 +190,6 @@ suite('data binding and mapping', () => {
     t.assert.strictEqual(setup, undefined);
 
     [
-      undefined,
       () => {},
       Symbol(),
       /foo/,