From 799f4149c58f5f648e2d2d1b4796859849dff93a Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Mon, 26 May 2025 16:04:54 -0300 Subject: [PATCH 1/3] bind/js: Add bind method --- .../__test__/better-sqlite3.spec.mjs | 19 +++++++++ bindings/javascript/__test__/limbo.spec.mjs | 20 +++++++++- bindings/javascript/src/lib.rs | 40 ++++++++++++++----- bindings/javascript/wrapper.js | 10 +++++ 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/bindings/javascript/__test__/better-sqlite3.spec.mjs b/bindings/javascript/__test__/better-sqlite3.spec.mjs index 94f3570dd..998155562 100644 --- a/bindings/javascript/__test__/better-sqlite3.spec.mjs +++ b/bindings/javascript/__test__/better-sqlite3.spec.mjs @@ -58,6 +58,25 @@ test("Test pragma", async (t) => { t.deepEqual(typeof db.pragma("cache_size", { simple: true }), "number"); }); +test("Test bind()", async (t) => { + const [db] = await connect(":memory:"); + db.prepare("CREATE TABLE users (name TEXT, age INTEGER)").run(); + db.prepare("INSERT INTO users (name, age) VALUES (?, ?)").run("Alice", 42); + let stmt = db.prepare("SELECT * FROM users WHERE name = ?").bind("Alice"); + + for (const row of stmt.iterate()) { + t.truthy(row.name); + t.true(typeof row.age === "number"); + } + + t.throws( + () => { + db.bind("Bob"); + }, + { instanceOf: Error }, + ); +}); + const connect = async (path) => { const db = new Database(path); return [db]; diff --git a/bindings/javascript/__test__/limbo.spec.mjs b/bindings/javascript/__test__/limbo.spec.mjs index 46047ab04..9740064c5 100644 --- a/bindings/javascript/__test__/limbo.spec.mjs +++ b/bindings/javascript/__test__/limbo.spec.mjs @@ -1,5 +1,4 @@ import test from "ava"; -import fs from "fs"; import { Database } from "../wrapper.js"; @@ -72,6 +71,25 @@ test("Test pragma", async (t) => { t.true(typeof db.pragma("cache_size", { simple: true }) === "number"); }); +test("Test bind()", async (t) => { + const [db] = await connect(":memory:"); + db.prepare("CREATE TABLE users (name TEXT, age INTEGER)").run(); + db.prepare("INSERT INTO users (name, age) VALUES (?, ?)").run("Alice", 42); + let stmt = db.prepare("SELECT * FROM users WHERE name = ?").bind("Alice"); + + for (const row of stmt.iterate()) { + t.truthy(row.name); + t.true(typeof row.age === "number"); + } + + t.throws( + () => { + db.bind("Bob"); + }, + { instanceOf: Error }, + ); +}); + const connect = async (path) => { const db = new Database(path); return [db]; diff --git a/bindings/javascript/src/lib.rs b/bindings/javascript/src/lib.rs index 7d84d6cc3..7b79fef86 100644 --- a/bindings/javascript/src/lib.rs +++ b/bindings/javascript/src/lib.rs @@ -170,8 +170,8 @@ impl Database { } } -// TODO: Add the (parent) 'database' property #[napi] +#[derive(Clone)] pub struct Statement { // TODO: implement each property when core supports it // #[napi(able = false)] @@ -183,8 +183,9 @@ pub struct Statement { #[napi(writable = false)] pub source: String, - toggle: bool, database: Database, + pluck: bool, + binded: bool, inner: Rc>, } @@ -195,7 +196,8 @@ impl Statement { inner: Rc::new(inner), database, source, - toggle: false, + pluck: false, + binded: false, } } @@ -321,29 +323,49 @@ impl Statement { } #[napi] - pub fn pluck(&mut self, toggle: Option) { - if let Some(false) = toggle { - self.toggle = false; + pub fn pluck(&mut self, pluck: Option) { + if let Some(false) = pluck { + self.pluck = false; } - self.toggle = true; + self.pluck = true; } #[napi] pub fn expand() { todo!() } + #[napi] pub fn raw() { todo!() } + #[napi] pub fn columns() { todo!() } + #[napi] - pub fn bind() { - todo!() + pub fn bind(&mut self, args: Option>) -> napi::Result { + let mut stmt = self.inner.borrow_mut(); + stmt.reset(); + if self.binded { + return Err(napi::Error::new( + napi::Status::InvalidArg, + "This statement already has bound parameters", + )); + } + if let Some(args) = args { + for (i, elem) in args.into_iter().enumerate() { + let value = from_js_value(elem)?; + stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); + } + } + + self.binded = true; + + Ok(self.clone()) } } diff --git a/bindings/javascript/wrapper.js b/bindings/javascript/wrapper.js index 0eb18e3da..c25b7bc97 100644 --- a/bindings/javascript/wrapper.js +++ b/bindings/javascript/wrapper.js @@ -224,6 +224,16 @@ class Statement { columns() { return this.stmt.columns(); } + + /** + * Binds the given parameters to the statement _permanently_ + * + * @param bindParameters - The bind parameters for binding the statement. + * @returns this - Statement with binded parameters + */ + bind(...bindParameters) { + return this.stmt.bind(bindParameters.flat()); + } } module.exports.Database = Database; From b60fd81995d39f11c5afe8ddf239195dbf9e1fb9 Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Mon, 26 May 2025 16:45:33 -0300 Subject: [PATCH 2/3] bind/js: Reduce boilerplate of binding variables and checking Statement's state --- bindings/javascript/src/lib.rs | 66 +++++++++++++--------------------- 1 file changed, 24 insertions(+), 42 deletions(-) diff --git a/bindings/javascript/src/lib.rs b/bindings/javascript/src/lib.rs index 7b79fef86..7d5afedc2 100644 --- a/bindings/javascript/src/lib.rs +++ b/bindings/javascript/src/lib.rs @@ -203,15 +203,7 @@ impl Statement { #[napi] pub fn get(&self, env: Env, args: Option>) -> napi::Result { - let mut stmt = self.inner.borrow_mut(); - stmt.reset(); - - if let Some(args) = args { - for (i, elem) in args.into_iter().enumerate() { - let value = from_js_value(elem)?; - stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); - } - } + let mut stmt = self.check_and_bind(args)?; let step = stmt.step().map_err(into_napi_error)?; match step { @@ -236,14 +228,7 @@ impl Statement { // TODO: Return Info object (https://github.com/WiseLibs/better-sqlite3/blob/master/docs/api.md#runbindparameters---object) #[napi] pub fn run(&self, env: Env, args: Option>) -> napi::Result { - let mut stmt = self.inner.borrow_mut(); - stmt.reset(); - if let Some(args) = args { - for (i, elem) in args.into_iter().enumerate() { - let value = from_js_value(elem)?; - stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); - } - } + let stmt = self.check_and_bind(args)?; self.internal_all(env, stmt) } @@ -254,14 +239,7 @@ impl Statement { env: Env, args: Option>, ) -> napi::Result { - let mut stmt = self.inner.borrow_mut(); - stmt.reset(); - if let Some(args) = args { - for (i, elem) in args.into_iter().enumerate() { - let value = from_js_value(elem)?; - stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); - } - } + self.check_and_bind(args)?; Ok(IteratorStatement { stmt: Rc::clone(&self.inner), @@ -272,14 +250,7 @@ impl Statement { #[napi] pub fn all(&self, env: Env, args: Option>) -> napi::Result { - let mut stmt = self.inner.borrow_mut(); - stmt.reset(); - if let Some(args) = args { - for (i, elem) in args.into_iter().enumerate() { - let value = from_js_value(elem)?; - stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); - } - } + let stmt = self.check_and_bind(args)?; self.internal_all(env, stmt) } @@ -348,24 +319,35 @@ impl Statement { #[napi] pub fn bind(&mut self, args: Option>) -> napi::Result { + self.check_and_bind(args)?; + self.binded = true; + + Ok(self.clone()) + } + + /// Check if the Statement is already binded by the `bind()` method + /// and bind values do variables. The expected type for args is `Option>` + fn check_and_bind( + &self, + args: Option>, + ) -> napi::Result> { let mut stmt = self.inner.borrow_mut(); stmt.reset(); - if self.binded { - return Err(napi::Error::new( - napi::Status::InvalidArg, - "This statement already has bound parameters", - )); - } if let Some(args) = args { + if self.binded { + return Err(napi::Error::new( + napi::Status::InvalidArg, + "This statement already has bound parameters", + )); + } + for (i, elem) in args.into_iter().enumerate() { let value = from_js_value(elem)?; stmt.bind_at(NonZeroUsize::new(i + 1).unwrap(), value); } } - self.binded = true; - - Ok(self.clone()) + Ok(stmt) } } From b012d07aa3287feb457947e71a101d89c9d6c8fe Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Mon, 26 May 2025 17:14:33 -0300 Subject: [PATCH 3/3] bind/js: Apply pluck's logic to all methods --- bindings/javascript/__test__/limbo.spec.mjs | 17 +++++++++++++++-- bindings/javascript/src/lib.rs | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/bindings/javascript/__test__/limbo.spec.mjs b/bindings/javascript/__test__/limbo.spec.mjs index 9740064c5..b7ba2e6fc 100644 --- a/bindings/javascript/__test__/limbo.spec.mjs +++ b/bindings/javascript/__test__/limbo.spec.mjs @@ -65,13 +65,13 @@ test("Empty prepared statement should throw", async (t) => { ); }); -test("Test pragma", async (t) => { +test("Test pragma()", async (t) => { const [db] = await connect(":memory:"); t.true(typeof db.pragma("cache_size")[0].cache_size === "number"); t.true(typeof db.pragma("cache_size", { simple: true }) === "number"); }); -test("Test bind()", async (t) => { +test("Statement binded with bind() shouldn't be binded again", async (t) => { const [db] = await connect(":memory:"); db.prepare("CREATE TABLE users (name TEXT, age INTEGER)").run(); db.prepare("INSERT INTO users (name, age) VALUES (?, ?)").run("Alice", 42); @@ -90,6 +90,19 @@ test("Test bind()", async (t) => { ); }); +test("Test pluck(): Rows should only have the values of the first column", async (t) => { + const [db] = await connect(":memory:"); + db.prepare("CREATE TABLE users (name TEXT, age INTEGER)").run(); + db.prepare("INSERT INTO users (name, age) VALUES (?, ?)").run("Alice", 42); + db.prepare("INSERT INTO users (name, age) VALUES (?, ?)").run("Bob", 24); + let stmt = db.prepare("SELECT * FROM users").pluck(); + + for (const row of stmt.iterate()) { + t.truthy(row.name); + t.true(typeof row.age === "undefined"); + } +}); + const connect = async (path) => { const db = new Database(path); return [db]; diff --git a/bindings/javascript/src/lib.rs b/bindings/javascript/src/lib.rs index 7d5afedc2..a46dc4f01 100644 --- a/bindings/javascript/src/lib.rs +++ b/bindings/javascript/src/lib.rs @@ -214,6 +214,10 @@ impl Statement { let key = stmt.get_column_name(idx); let js_value = to_js_value(&env, value); obj.set_named_property(&key, js_value)?; + + if self.pluck { + return Ok(obj.into_unknown()); + } } Ok(obj.into_unknown()) } @@ -245,6 +249,7 @@ impl Statement { stmt: Rc::clone(&self.inner), database: self.database.clone(), env, + plucked: self.pluck, }) } @@ -271,6 +276,10 @@ impl Statement { let key = stmt.get_column_name(idx); let js_value = to_js_value(&env, value); obj.set_named_property(&key, js_value)?; + + if self.pluck { + break; + } } results.set_element(index, obj)?; index += 1; @@ -356,6 +365,7 @@ pub struct IteratorStatement { stmt: Rc>, database: Database, env: Env, + plucked: bool, } impl Generator for IteratorStatement { @@ -377,6 +387,10 @@ impl Generator for IteratorStatement { let key = stmt.get_column_name(idx); let js_value = to_js_value(&self.env, value); js_row.set_named_property(&key, js_value).ok()?; + + if self.plucked { + break; + } } Some(js_row)