Skip to content

sqlite: statement.setReadNullAsUndefined() #59462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+262 −20
Open

sqlite: statement.setReadNullAsUndefined() #59462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1c3702c
src: add setReadNullAsUndefined() to StatementSync in node_sqlite.cc …
jac0bandres Aug 13, 2025
f4a6327
Merge branch 'nodejs:main' into dev
jac0bandres Aug 13, 2025
06e050f
sqlite: add statement.setReadNullAsUndefined, add options.readNullAsU…
jac0bandres Aug 14, 2025
8af485a
sqlite: add statement.setReadNullAsUndefined, add options.readNullAsU…
jac0bandres Aug 14, 2025
f248a9e
sqlite: add statement.setReadNullAsUndefined, add options.readNullAsU…
jac0bandres Aug 14, 2025
b62aed9
sqlite: add statement.setReadNullAsUndefined, add options.readNullAsU…
jac0bandres Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/env_properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@
V(read_host_object_string, "_readHostObject") \
V(readable_string, "readable") \
V(read_bigints_string, "readBigInts") \
V(read_null_as_undefined_string, "readNullAsUndefined") \
V(reason_string, "reason") \
V(refresh_string, "refresh") \
V(regexp_string, "regexp") \
Expand Down
123 changes: 107 additions & 16 deletions src/node_sqlite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ using v8::MaybeLocal;
using v8::Name;
using v8::NewStringType;
using v8::Null;
using v8::Undefined;
using v8::Number;
using v8::Object;
using v8::Promise;
Expand All @@ -67,7 +68,10 @@ using v8::Value;
} \
} while (0)

#define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \
#define SQLITE_VALUE_TO_JS(from, isolate, \
use_big_int_args, \
use_null_as_undefined_, \
result, ...) \
do { \
switch (sqlite3_##from##_type(__VA_ARGS__)) { \
case SQLITE_INTEGER: { \
Expand Down Expand Up @@ -96,7 +100,11 @@ using v8::Value;
break; \
} \
case SQLITE_NULL: { \
(result) = Null((isolate)); \
if ((use_null_as_undefined_)) { \
(result) = Undefined((isolate)); \
} else { \
(result) = Null((isolate)); \
} \
Comment on lines +103 to +107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LiviaMedeiros when the value is undefined, should it be included as a key in the returned object?

V8 optimizes creation of the objects with similar shape but I don't know if this applies to objects created via C++ directly, if not, maybe could be worthy/faster just exclude this field.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance-wise, i don't know. On huge amounts of rows and sparse data, i would expect hidden class to be faster at the cost of extra memory consumption; but without benchmarking it's just a guess.

However, this difference is observable in userland, which outweights microoptimizations. I think, explicit undefined is better:
If we get rows as objects using SELECT * FROM ..., we should be able to check what * was resolved to.
If we use returnArrays: true or statement.setReturnArrays(true) and keep it consistent with objects, omitting undefined means having sparse arrays which are counterintuitive.

If there is significant performance benefit from excluding fields, we can consider more explicit useNullAsEmpty. Or changing this from boolean to useNullAs(enum) while node:sqlite is still experimental.

break; \
} \
case SQLITE_BLOB: { \
Expand Down Expand Up @@ -254,13 +262,15 @@ class CustomAggregate {
explicit CustomAggregate(Environment* env,
DatabaseSync* db,
bool use_bigint_args,
bool use_null_as_undefined_args,
Local<Value> start,
Local<Function> step_fn,
Local<Function> inverse_fn,
Local<Function> result_fn)
: env_(env),
db_(db),
use_bigint_args_(use_bigint_args),
use_null_as_undefined_args_(use_null_as_undefined_args),
start_(env->isolate(), start),
step_fn_(env->isolate(), step_fn),
inverse_fn_(env->isolate(), inverse_fn),
Expand Down Expand Up @@ -310,7 +320,12 @@ class CustomAggregate {
for (int i = 0; i < argc; ++i) {
sqlite3_value* value = argv[i];
MaybeLocal<Value> js_val;
SQLITE_VALUE_TO_JS(value, isolate, self->use_bigint_args_, js_val, value);
SQLITE_VALUE_TO_JS(value,
isolate,
self->use_bigint_args_,
self->use_null_as_undefined_args_,
js_val,
value);
if (js_val.IsEmpty()) {
// Ignore the SQLite error because a JavaScript exception is pending.
self->db_->SetIgnoreNextSQLiteError(true);
Expand Down Expand Up @@ -418,6 +433,7 @@ class CustomAggregate {
Environment* env_;
DatabaseSync* db_;
bool use_bigint_args_;
bool use_null_as_undefined_args_;
Global<Value> start_;
Global<Function> step_fn_;
Global<Function> inverse_fn_;
Expand Down Expand Up @@ -591,11 +607,14 @@ class BackupJob : public ThreadPoolWork {
UserDefinedFunction::UserDefinedFunction(Environment* env,
Local<Function> fn,
DatabaseSync* db,
bool use_bigint_args)
bool use_bigint_args,
bool use_null_as_undefined_args
)
: env_(env),
fn_(env->isolate(), fn),
db_(db),
use_bigint_args_(use_bigint_args) {}
use_bigint_args_(use_bigint_args),
use_null_as_undefined_args_(use_null_as_undefined_args) {}

UserDefinedFunction::~UserDefinedFunction() {}

Expand All @@ -613,7 +632,12 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
for (int i = 0; i < argc; ++i) {
sqlite3_value* value = argv[i];
MaybeLocal<Value> js_val = MaybeLocal<Value>();
SQLITE_VALUE_TO_JS(value, isolate, self->use_bigint_args_, js_val, value);
SQLITE_VALUE_TO_JS(value,
isolate,
self->use_bigint_args_,
self->use_null_as_undefined_args_,
js_val,
value);
if (js_val.IsEmpty()) {
// Ignore the SQLite error because a JavaScript exception is pending.
self->db_->SetIgnoreNextSQLiteError(true);
Expand Down Expand Up @@ -981,6 +1005,21 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
}
}

Local<Value> read_null_as_undefined_v;
if (options->Get(env->context(), env->read_null_as_undefined_string())
.ToLocal(&read_null_as_undefined_v)) {
if (!read_null_as_undefined_v->IsUndefined()) {
if (!read_null_as_undefined_v->IsBoolean()) {
THROW_ERR_INVALID_ARG_TYPE(
env->isolate(),
R"(The "options.readNullAsUndefined" argument must be a boolean.)");
return;
}
open_config.set_use_null_as_undefined(read_null_as_undefined_v
.As<Boolean>()->Value());
}
}

Local<Value> return_arrays_v;
if (options->Get(env->context(), env->return_arrays_string())
.ToLocal(&return_arrays_v)) {
Expand Down Expand Up @@ -1126,6 +1165,7 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo<Value>& args) {

int fn_index = args.Length() < 3 ? 1 : 2;
bool use_bigint_args = false;
bool use_null_as_undefined_args = false;
bool varargs = false;
bool deterministic = false;
bool direct_only = false;
Expand Down Expand Up @@ -1234,7 +1274,11 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo<Value>& args) {
}

UserDefinedFunction* user_data =
new UserDefinedFunction(env, fn, db, use_bigint_args);
new UserDefinedFunction(env,
fn,
db,
use_bigint_args,
use_null_as_undefined_args);
int text_rep = SQLITE_UTF8;

if (deterministic) {
Expand Down Expand Up @@ -1323,9 +1367,11 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
}

bool use_bigint_args = false;
bool use_null_as_undefined_args = false;
bool varargs = false;
bool direct_only = false;
Local<Value> use_bigint_args_v;
Local<Value> use_null_as_undefined_args_v;
Local<Function> inverseFunc = Local<Function>();
if (!options
->Get(env->context(),
Expand All @@ -1344,6 +1390,23 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
use_bigint_args = use_bigint_args_v.As<Boolean>()->Value();
}

if (!options
->Get(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "useNullAsUndefined"))
.ToLocal(&use_null_as_undefined_args_v)) {
return;
}

if (!use_null_as_undefined_args_v->IsUndefined()) {
if (!use_null_as_undefined_args_v->IsBoolean()) {
THROW_ERR_INVALID_ARG_TYPE(
env->isolate(),
"The \"options.useNullAsUndefined\" argument must be a boolean.");
return;
}
use_null_as_undefined_args = use_bigint_args_v.As<Boolean>()->Value();
}

Local<Value> varargs_v;
if (!options
->Get(env->context(),
Expand Down Expand Up @@ -1429,13 +1492,15 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
*name,
argc,
text_rep,
new CustomAggregate(env,
db,
use_bigint_args,
start_v,
stepFunction,
inverseFunc,
resultFunction),
new CustomAggregate(
env,
db,
use_bigint_args,
use_null_as_undefined_args,
start_v,
stepFunction,
inverseFunc,
resultFunction),
CustomAggregate::xStep,
CustomAggregate::xFinal,
xValue,
Expand Down Expand Up @@ -1999,8 +2064,13 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
MaybeLocal<Value> StatementSync::ColumnToValue(const int column) {
Isolate* isolate = env()->isolate();
MaybeLocal<Value> js_val = MaybeLocal<Value>();
SQLITE_VALUE_TO_JS(
column, isolate, use_big_ints_, js_val, statement_, column);
SQLITE_VALUE_TO_JS(column,
isolate,
use_big_ints_,
use_null_as_undefined_,
js_val,
statement_,
column);
return js_val;
}

Expand Down Expand Up @@ -2378,6 +2448,24 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo<Value>& args) {
stmt->use_big_ints_ = args[0]->IsTrue();
}

void StatementSync::SetReadNullAsUndefined(
const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(
env, stmt->IsFinalized(), "statement has been finalized");

if (!args[0]->IsBoolean()) {
THROW_ERR_INVALID_ARG_TYPE(
env->isolate(),
"The \"readNullAsUndefined\" argument must be a boolean.");
return;
}

stmt->use_null_as_undefined_ = args[0]->IsTrue();
}

void StatementSync::SetReturnArrays(const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Expand Down Expand Up @@ -2447,6 +2535,9 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
tmpl,
"setAllowUnknownNamedParameters",
StatementSync::SetAllowUnknownNamedParameters);
SetProtoMethod(
isolate, tmpl, "setReadNullAsUndefined",
StatementSync::SetReadNullAsUndefined);
SetProtoMethod(
isolate, tmpl, "setReadBigInts", StatementSync::SetReadBigInts);
SetProtoMethod(
Expand Down
24 changes: 22 additions & 2 deletions src/node_sqlite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ class DatabaseOpenConfiguration {

inline bool get_use_big_ints() const { return use_big_ints_; }

inline void set_use_null_as_undefined(bool flag) {
use_null_as_undefined_ = flag;
}

inline bool get_use_null_as_undefined() const {
return use_null_as_undefined_;
}

inline void set_return_arrays(bool flag) { return_arrays_ = flag; }

inline bool get_return_arrays() const { return return_arrays_; }
Expand Down Expand Up @@ -70,6 +78,7 @@ class DatabaseOpenConfiguration {
bool enable_dqs_ = false;
int timeout_ = 0;
bool use_big_ints_ = false;
bool use_null_as_undefined_ = false;
bool return_arrays_ = false;
bool allow_bare_named_params_ = true;
bool allow_unknown_named_params_ = false;
Expand Down Expand Up @@ -110,7 +119,12 @@ class DatabaseSync : public BaseObject {
void FinalizeBackups();
void UntrackStatement(StatementSync* statement);
bool IsOpen();
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
bool use_big_ints() const {
return open_config_.get_use_big_ints();
}
bool use_null_as_undefined() const {
return open_config_.get_use_null_as_undefined();
}
bool return_arrays() const { return open_config_.get_return_arrays(); }
bool allow_bare_named_params() const {
return open_config_.get_allow_bare_named_params();
Expand Down Expand Up @@ -173,6 +187,8 @@ class StatementSync : public BaseObject {
static void SetAllowUnknownNamedParameters(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetReadBigInts(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetReadNullAsUndefined(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetReturnArrays(const v8::FunctionCallbackInfo<v8::Value>& args);
void Finalize();
bool IsFinalized();
Expand All @@ -186,6 +202,7 @@ class StatementSync : public BaseObject {
sqlite3_stmt* statement_;
bool return_arrays_ = false;
bool use_big_ints_;
bool use_null_as_undefined_ = false;
bool allow_bare_named_params_;
bool allow_unknown_named_params_;
std::optional<std::map<std::string, std::string>> bare_named_params_;
Expand Down Expand Up @@ -253,7 +270,9 @@ class UserDefinedFunction {
UserDefinedFunction(Environment* env,
v8::Local<v8::Function> fn,
DatabaseSync* db,
bool use_bigint_args);
bool use_bigint_args,
bool use_null_as_undefined_args
);
~UserDefinedFunction();
static void xFunc(sqlite3_context* ctx, int argc, sqlite3_value** argv);
static void xDestroy(void* self);
Expand All @@ -263,6 +282,7 @@ class UserDefinedFunction {
v8::Global<v8::Function> fn_;
DatabaseSync* db_;
bool use_bigint_args_;
bool use_null_as_undefined_args_;
};

} // namespace sqlite
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-sqlite-aggregate-function.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ describe('DatabaseSync.prototype.aggregate()', () => {
});
});

test('throws if options.readNullAsUndefined is provided but is not a boolean', (t) => {
t.assert.throws(() => {
new DatabaseSync('foo', { readNullAsUndefined: 42 });
}, {
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "options.readNullAsUndefined" argument must be a boolean.',
});
});

test('throws if options.varargs is not a boolean', (t) => {
t.assert.throws(() => {
db.aggregate('sum', {
Expand Down
Loading
Loading