Skip to content

Commit d4c9b5c

Browse files
thisalihassanaduh95
authored andcommitted
sqlite: avoid extra copy for large text binds
When binding UTF-8 strings to prepared statements, transfer ownership of malloc-backed Utf8Value buffers to SQLite to avoid an extra copy for large strings. Use sqlite3_bind_blob64() when binding BLOB parameters. PR-URL: #61580 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
1 parent 28905b9 commit d4c9b5c

File tree

2 files changed

+61
-8
lines changed

2 files changed

+61
-8
lines changed

src/node_sqlite.cc

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,20 +2261,38 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
22612261
// Dates could be supported by converting them to numbers. However, there
22622262
// would not be a good way to read the values back from SQLite with the
22632263
// original type.
2264+
Isolate* isolate = env()->isolate();
22642265
int r;
22652266
if (value->IsNumber()) {
2266-
double val = value.As<Number>()->Value();
2267+
const double val = value.As<Number>()->Value();
22672268
r = sqlite3_bind_double(statement_, index, val);
22682269
} else if (value->IsString()) {
2269-
Utf8Value val(env()->isolate(), value.As<String>());
2270-
r = sqlite3_bind_text(
2271-
statement_, index, *val, val.length(), SQLITE_TRANSIENT);
2270+
Utf8Value val(isolate, value.As<String>());
2271+
if (val.IsAllocated()) {
2272+
// Avoid an extra SQLite copy for large strings by transferring ownership
2273+
// of the malloc()'d buffer to SQLite.
2274+
char* data = *val;
2275+
const sqlite3_uint64 length = static_cast<sqlite3_uint64>(val.length());
2276+
val.Release();
2277+
r = sqlite3_bind_text64(
2278+
statement_, index, data, length, std::free, SQLITE_UTF8);
2279+
} else {
2280+
r = sqlite3_bind_text64(statement_,
2281+
index,
2282+
*val,
2283+
static_cast<sqlite3_uint64>(val.length()),
2284+
SQLITE_TRANSIENT,
2285+
SQLITE_UTF8);
2286+
}
22722287
} else if (value->IsNull()) {
22732288
r = sqlite3_bind_null(statement_, index);
22742289
} else if (value->IsArrayBufferView()) {
22752290
ArrayBufferViewContents<uint8_t> buf(value);
2276-
r = sqlite3_bind_blob(
2277-
statement_, index, buf.data(), buf.length(), SQLITE_TRANSIENT);
2291+
r = sqlite3_bind_blob64(statement_,
2292+
index,
2293+
buf.data(),
2294+
static_cast<sqlite3_uint64>(buf.length()),
2295+
SQLITE_TRANSIENT);
22782296
} else if (value->IsBigInt()) {
22792297
bool lossless;
22802298
int64_t as_int = value.As<BigInt>()->Int64Value(&lossless);
@@ -2285,13 +2303,13 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
22852303
r = sqlite3_bind_int64(statement_, index, as_int);
22862304
} else {
22872305
THROW_ERR_INVALID_ARG_TYPE(
2288-
env()->isolate(),
2306+
isolate,
22892307
"Provided value cannot be bound to SQLite parameter %d.",
22902308
index);
22912309
return false;
22922310
}
22932311

2294-
CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false);
2312+
CHECK_ERROR_OR_THROW(isolate, db_.get(), r, SQLITE_OK, false);
22952313
return true;
22962314
}
22972315

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,41 @@ suite('data binding and mapping', () => {
8282
});
8383
});
8484

85+
test('large strings are bound correctly', (t) => {
86+
const db = new DatabaseSync(nextDb());
87+
t.after(() => { db.close(); });
88+
const setup = db.exec(
89+
'CREATE TABLE data(key INTEGER PRIMARY KEY, text TEXT) STRICT;'
90+
);
91+
t.assert.strictEqual(setup, undefined);
92+
93+
t.assert.deepStrictEqual(
94+
db.prepare('INSERT INTO data (key, text) VALUES (?, ?)').run(1, ''),
95+
{ changes: 1, lastInsertRowid: 1 },
96+
);
97+
98+
const update = db.prepare('UPDATE data SET text = ? WHERE key = 1');
99+
100+
// > 1024 bytes so `Utf8Value` uses heap storage internally.
101+
const largeAscii = 'a'.repeat(8 * 1024);
102+
// Force a non-one-byte string path through UTF-8 conversion.
103+
const largeUnicode = '\u2603'.repeat(2048);
104+
105+
const res = update.run(largeAscii);
106+
t.assert.strictEqual(res.changes, 1);
107+
108+
t.assert.strictEqual(
109+
db.prepare('SELECT text FROM data WHERE key = 1').get().text,
110+
largeAscii,
111+
);
112+
113+
t.assert.strictEqual(update.run(largeUnicode).changes, 1);
114+
t.assert.strictEqual(
115+
db.prepare('SELECT text FROM data WHERE key = 1').get().text,
116+
largeUnicode,
117+
);
118+
});
119+
85120
test('unsupported data types', (t) => {
86121
const db = new DatabaseSync(nextDb());
87122
t.after(() => { db.close(); });

0 commit comments

Comments
 (0)