Skip to content

fix(node:sqlite): Fix Named parameters #28154

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.

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

Merged
merged 6 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
124 changes: 118 additions & 6 deletions ext/node/ops/sqlite/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::rc::Rc;

use deno_core::op2;
use deno_core::v8;
use deno_core::v8::GetPropertyNamesArgs;
use deno_core::GarbageCollected;
use libsqlite3_sys as ffi;
use serde::Serialize;
Expand Down Expand Up @@ -217,6 +218,7 @@ impl StatementSync {

if let Some(params) = params {
let len = params.length();
let mut param_count = 0;
for i in 0..len {
let value = params.get(i);

Expand All @@ -226,8 +228,9 @@ impl StatementSync {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_double(raw, i + 1, value);
ffi::sqlite3_bind_double(raw, param_count + 1, value);
}
param_count += 1
} else if value.is_string() {
let value = value.to_rust_string_lossy(scope);

Expand All @@ -238,18 +241,20 @@ impl StatementSync {
unsafe {
ffi::sqlite3_bind_text(
raw,
i + 1,
param_count + 1,
value.as_ptr() as *const _,
value.len() as i32,
ffi::SQLITE_TRANSIENT(),
);
}
param_count += 1;
} else if value.is_null() {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_null(raw, i + 1);
ffi::sqlite3_bind_null(raw, param_count + 1);
}
param_count += 1;
} else if value.is_array_buffer_view() {
let value: v8::Local<v8::ArrayBufferView> = value.try_into().unwrap();
let data = value.data();
Expand All @@ -262,12 +267,13 @@ impl StatementSync {
unsafe {
ffi::sqlite3_bind_blob(
raw,
i + 1,
param_count + 1,
data,
size as i32,
ffi::SQLITE_TRANSIENT(),
);
}
param_count += 1
} else if value.is_big_int() {
let value: v8::Local<v8::BigInt> = value.try_into().unwrap();
let (as_int, lossless) = value.i64_value();
Expand All @@ -280,7 +286,32 @@ impl StatementSync {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_int64(raw, i + 1, as_int);
ffi::sqlite3_bind_int64(raw, param_count + 1, as_int);
}
param_count += 1;
} else if value.is_object() {
let value: v8::Local<v8::Object> = value.try_into().unwrap();
let maybe_keys =
value.get_property_names(scope, GetPropertyNamesArgs::default());

if let Some(keys) = maybe_keys {
let length = keys.length();
for i in 0..length {
if let Some(key) = keys.get_index(scope, i) {
if let Some(key_str) = key.to_string(scope) {
let key_str = key_str.to_rust_string_lossy(scope);
if let Some(value) = value.get(scope, key) {
self.bind_params_object(
scope,
key_str,
value,
param_count,
)?;
param_count += 1;
}
}
}
}
}
} else {
return Err(SqliteError::FailedBind("Unsupported type"));
Expand All @@ -290,6 +321,88 @@ impl StatementSync {

Ok(())
}

//helper function that binds the object attribute to named param
fn bind_params_object(
&self,
scope: &mut v8::HandleScope,
key: String,
value: v8::Local<v8::Value>,
count: i32,
) -> Result<(), SqliteError> {
let raw = self.inner;

// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_parameter_index(raw, key.as_ptr() as *const _);
}

if value.is_number() {
let value = value.number_value(scope).unwrap();

// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_double(raw, count + 1, value);
}
} else if value.is_string() {
let value = value.to_rust_string_lossy(scope);

// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_text(
raw,
count + 1,
value.as_ptr() as *const _,
value.len() as i32,
ffi::SQLITE_TRANSIENT(),
);
}
} else if value.is_null() {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_null(raw, count + 1);
}
} else if value.is_array_buffer_view() {
let value: v8::Local<v8::ArrayBufferView> = value.try_into().unwrap();
let data = value.data();
let size = value.byte_length();

// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
//
// SQLITE_TRANSIENT is used to indicate that SQLite should make a copy of the data.
unsafe {
ffi::sqlite3_bind_blob(
raw,
count + 1,
data,
size as i32,
ffi::SQLITE_TRANSIENT(),
);
}
} else if value.is_big_int() {
let value: v8::Local<v8::BigInt> = value.try_into().unwrap();
let (as_int, lossless) = value.i64_value();
if !lossless {
return Err(SqliteError::FailedBind(
"BigInt value is too large to bind",
));
}

// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_int64(raw, count + 1, as_int);
}
} else {
return Err(SqliteError::FailedBind("Unsupported type"));
}
Ok(())
}
}

// Represents a single prepared statement. Cannot be initialized directly via constructor.
Expand Down Expand Up @@ -542,7 +655,6 @@ impl StatementSync {
.to_string_lossy()
.into_owned();
ffi::sqlite3_free(raw as _);

Ok(sql)
}
}
Expand Down
19 changes: 19 additions & 0 deletions tests/unit_node/sqlite_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,25 @@ CREATE TABLE two(id int PRIMARY KEY) STRICT;`);
db.close();
});

Deno.test("[node/sqlite] query should handle mixed positional and named parameters", () => {
const db = new DatabaseSync(":memory:");
db.exec(`CREATE TABLE one(variable1 TEXT, variable2 INT, variable3 INT)`);
db.exec(
`INSERT INTO one (variable1, variable2, variable3) VALUES ("test", 1 , 2);`,
);

const query = "SELECT * FROM one WHERE variable1=:var1 AND variable2=:var2 ";
const result = db.prepare(query).all({ var1: "test", var2: 1 });
assertEquals(result, [{
__proto__: null,
variable1: "test",
variable2: 1,
variable3: 2,
}]);

db.close();
});

Deno.test("[node/sqlite] StatementSync#iterate", () => {
const db = new DatabaseSync(":memory:");
const stmt = db.prepare("SELECT 1 UNION ALL SELECT 2 UNION ALL SELECT 3");
Expand Down