Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d4c9b5c

Browse filesBrowse files
thisalihassanaduh95
thisalihassan
authored and
aduh95
committed
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
Copy full SHA for d4c9b5c

2 files changed

+61-8Lines changed: 61 additions & 8 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/node_sqlite.cc‎

Copy file name to clipboardExpand all lines: src/node_sqlite.cc
+26-8Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-data-types.js
+35Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.