Skip to content

Commit bf3ff4b

Browse files
thisalihassanjuanarbol
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 783ea7d commit bf3ff4b

2 files changed

Lines changed: 61 additions & 8 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1957,20 +1957,38 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
19571957
// Dates could be supported by converting them to numbers. However, there
19581958
// would not be a good way to read the values back from SQLite with the
19591959
// original type.
1960+
Isolate* isolate = env()->isolate();
19601961
int r;
19611962
if (value->IsNumber()) {
1962-
double val = value.As<Number>()->Value();
1963+
const double val = value.As<Number>()->Value();
19631964
r = sqlite3_bind_double(statement_, index, val);
19641965
} else if (value->IsString()) {
1965-
Utf8Value val(env()->isolate(), value.As<String>());
1966-
r = sqlite3_bind_text(
1967-
statement_, index, *val, val.length(), SQLITE_TRANSIENT);
1966+
Utf8Value val(isolate, value.As<String>());
1967+
if (val.IsAllocated()) {
1968+
// Avoid an extra SQLite copy for large strings by transferring ownership
1969+
// of the malloc()'d buffer to SQLite.
1970+
char* data = *val;
1971+
const sqlite3_uint64 length = static_cast<sqlite3_uint64>(val.length());
1972+
val.Release();
1973+
r = sqlite3_bind_text64(
1974+
statement_, index, data, length, std::free, SQLITE_UTF8);
1975+
} else {
1976+
r = sqlite3_bind_text64(statement_,
1977+
index,
1978+
*val,
1979+
static_cast<sqlite3_uint64>(val.length()),
1980+
SQLITE_TRANSIENT,
1981+
SQLITE_UTF8);
1982+
}
19681983
} else if (value->IsNull()) {
19691984
r = sqlite3_bind_null(statement_, index);
19701985
} else if (value->IsArrayBufferView()) {
19711986
ArrayBufferViewContents<uint8_t> buf(value);
1972-
r = sqlite3_bind_blob(
1973-
statement_, index, buf.data(), buf.length(), SQLITE_TRANSIENT);
1987+
r = sqlite3_bind_blob64(statement_,
1988+
index,
1989+
buf.data(),
1990+
static_cast<sqlite3_uint64>(buf.length()),
1991+
SQLITE_TRANSIENT);
19741992
} else if (value->IsBigInt()) {
19751993
bool lossless;
19761994
int64_t as_int = value.As<BigInt>()->Int64Value(&lossless);
@@ -1981,13 +1999,13 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
19811999
r = sqlite3_bind_int64(statement_, index, as_int);
19822000
} else {
19832001
THROW_ERR_INVALID_ARG_TYPE(
1984-
env()->isolate(),
2002+
isolate,
19852003
"Provided value cannot be bound to SQLite parameter %d.",
19862004
index);
19872005
return false;
19882006
}
19892007

1990-
CHECK_ERROR_OR_THROW(env()->isolate(), db_.get(), r, SQLITE_OK, false);
2008+
CHECK_ERROR_OR_THROW(isolate, db_.get(), r, SQLITE_OK, false);
19912009
return true;
19922010
}
19932011

‎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)