mirror of
https://github.com/nodejs/node.git
synced 2026-11-09 15:11:11 -05:00
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: https://github.com/nodejs/node/issues/61824 Refs: https://github.com/nodejs/node/pull/61472 Refs: https://github.com/nodejs/node/pull/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> PR-URL: https://github.com/nodejs/node/pull/65709 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit is contained in:
1 parent
345074e360
commit
41cb09f8a5
3 files changed
+117
-12
No files matched your search
+50
-9
@@ -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`
|
||||
@@ -1091,6 +1095,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.
|
||||
|
||||
@@ -1099,6 +1108,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.
|
||||
@@ -1114,7 +1126,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
|
||||
@@ -1187,6 +1200,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.
|
||||
@@ -1202,7 +1218,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
|
||||
@@ -1222,6 +1239,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.
|
||||
@@ -1237,7 +1257,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
|
||||
@@ -1266,6 +1287,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.
|
||||
@@ -1281,7 +1305,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
|
||||
@@ -1450,6 +1475,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.
|
||||
@@ -1460,7 +1488,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.
|
||||
|
||||
@@ -1475,6 +1504,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.
|
||||
@@ -1485,7 +1517,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.
|
||||
@@ -1500,6 +1533,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.
|
||||
@@ -1510,7 +1546,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.
|
||||
|
||||
@@ -1524,6 +1561,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.
|
||||
@@ -1534,7 +1574,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`.
|
||||
|
||||
|
||||
+3
-2
@@ -3158,7 +3158,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()) {
|
||||
@@ -3182,7 +3183,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()) {
|
||||
|
||||
@@ -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/,
|
||||
|
||||
Reference in new issue
Block a user