From f65d0807d19730718c4bae3ecc77d01ed2b69276 Mon Sep 17 00:00:00 2001 From: smith-xyz <51304449+smith-xyz@users.noreply.github.com> Date: Sun, 12 Feb 2023 14:12:32 -0500 Subject: [PATCH 1/5] add: expanded sql --- lib/sqlite3.d.ts | 6 +- src/macros.h | 10 ++- src/statement.cc | 20 +++++- src/statement.h | 4 +- test/expandedSql.test.js | 134 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 test/expandedSql.test.js diff --git a/lib/sqlite3.d.ts b/lib/sqlite3.d.ts index b27b0cf51..bc2e4a89d 100644 --- a/lib/sqlite3.d.ts +++ b/lib/sqlite3.d.ts @@ -76,7 +76,7 @@ export class Statement extends events.EventEmitter { finalize(callback?: (err: Error) => void): Database; - run(callback?: (err: Error | null) => void): this; + run(callback?: (this: RunResult, err: Error | null) => void): this; run(params: any, callback?: (this: RunResult, err: Error | null) => void): this; run(...params: any[]): this; @@ -91,6 +91,8 @@ export class Statement extends events.EventEmitter { each(callback?: (err: Error | null, row: any) => void, complete?: (err: Error | null, count: number) => void): this; each(params: any, callback?: (this: RunResult, err: Error | null, row: any) => void, complete?: (err: Error | null, count: number) => void): this; each(...params: any[]): this; + + readonly expandedSql: string; } export class Database extends events.EventEmitter { @@ -117,7 +119,7 @@ export class Database extends events.EventEmitter { exec(sql: string, callback?: (this: Statement, err: Error | null) => void): this; - prepare(sql: string, callback?: (this: Statement, err: Error | null) => void): Statement; + prepare(sql: string, callback?: (this: Statement, err: Error | null, expandedSql: string) => void): Statement; prepare(sql: string, params: any, callback?: (this: Statement, err: Error | null) => void): Statement; prepare(sql: string, ...params: any[]): Statement; diff --git a/src/macros.h b/src/macros.h index 344642d9d..4e3ccfa9e 100644 --- a/src/macros.h +++ b/src/macros.h @@ -165,6 +165,14 @@ inline bool OtherIsInt(Napi::Number source) { } \ sqlite3_mutex* name = sqlite3_db_mutex(stmt->db->_handle); +#define STATEMENT_EXPAND_SQL() \ + if (stmt->_handle != NULL) { \ + Napi::Env env = stmt->Env(); \ + Napi::Value expanded_sql = Napi::String::New(env, sqlite3_expanded_sql(stmt->_handle)); \ + Napi::Value key = Napi::String::New(env, "expandedSql"); \ + (stmt->Value()).Set(key, expanded_sql); \ + } + #define STATEMENT_END() \ assert(stmt->locked); \ assert(stmt->db->pending); \ @@ -210,6 +218,6 @@ inline bool OtherIsInt(Napi::Number source) { case SQLITE_BLOB: delete (Values::Blob*)(field); break; \ case SQLITE_NULL: delete (Values::Null*)(field); break; \ } \ - } + } #endif diff --git a/src/statement.cc b/src/statement.cc index f1b835ba2..03e7403c0 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -161,6 +161,8 @@ void Statement::Work_AfterPrepare(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_OK) { Error(baton.get()); stmt->Finalize_(); @@ -170,6 +172,7 @@ void Statement::Work_AfterPrepare(napi_env e, napi_status status, void* data) { if (!baton->callback.IsEmpty() && baton->callback.Value().IsFunction()) { Napi::Function cb = baton->callback.Value(); Napi::Value argv[] = { env.Null() }; + TRY_CATCH_CALL(stmt->Value(), cb, 1, argv); } } @@ -359,6 +362,7 @@ void Statement::Work_Bind(napi_env e, void* data) { STATEMENT_MUTEX(mtx); sqlite3_mutex_enter(mtx); stmt->Bind(baton->parameters); + sqlite3_mutex_leave(mtx); } @@ -369,6 +373,8 @@ void Statement::Work_AfterBind(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_OK) { Error(baton.get()); } @@ -436,6 +442,8 @@ void Statement::Work_AfterGet(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { Error(baton.get()); } @@ -510,6 +518,8 @@ void Statement::Work_AfterRun(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { Error(baton.get()); } @@ -518,8 +528,7 @@ void Statement::Work_AfterRun(napi_env e, napi_status status, void* data) { Napi::Function cb = baton->callback.Value(); if (IS_FUNCTION(cb)) { (stmt->Value()).Set(Napi::String::New(env, "lastID"), Napi::Number::New(env, baton->inserted_id)); - (stmt->Value()).Set( Napi::String::New(env, "changes"), Napi::Number::New(env, baton->changes)); - + (stmt->Value()).Set(Napi::String::New(env, "changes"), Napi::Number::New(env, baton->changes)); Napi::Value argv[] = { env.Null() }; TRY_CATCH_CALL(stmt->Value(), cb, 1, argv); } @@ -580,6 +589,8 @@ void Statement::Work_AfterAll(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_DONE) { Error(baton.get()); } @@ -752,10 +763,11 @@ void Statement::Work_AfterEach(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + if (stmt->status != SQLITE_DONE) { Error(baton.get()); } - STATEMENT_END(); } @@ -789,6 +801,8 @@ void Statement::Work_AfterReset(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(); + // Fire callbacks. Napi::Function cb = baton->callback.Value(); if (IS_FUNCTION(cb)) { diff --git a/src/statement.h b/src/statement.h index dec0015d1..2b8ae27e2 100644 --- a/src/statement.h +++ b/src/statement.h @@ -208,7 +208,7 @@ class Statement : public Napi::ObjectWrap { WORK_DEFINITION(Reset); Napi::Value Finalize_(const Napi::CallbackInfo& info); - + protected: static void Work_BeginPrepare(Database::Baton* baton); static void Work_Prepare(napi_env env, void* data); @@ -223,7 +223,7 @@ class Statement : public Napi::ObjectWrap { template inline Values::Field* BindParameter(const Napi::Value source, T pos); template T* Bind(const Napi::CallbackInfo& info, int start = 0, int end = -1); bool Bind(const Parameters ¶meters); - + static void GetRow(Row* row, sqlite3_stmt* stmt); static Napi::Value RowToJS(Napi::Env env, Row* row); void Schedule(Work_Callback callback, Baton* baton); diff --git a/test/expandedSql.test.js b/test/expandedSql.test.js new file mode 100644 index 000000000..29691b906 --- /dev/null +++ b/test/expandedSql.test.js @@ -0,0 +1,134 @@ +var sqlite3 = require('../lib/sqlite3'); +var assert = require('assert'); + +describe('expandedSql', function() { + var db; + before(function(done) { + db = new sqlite3.Database(':memory:'); + db.run("CREATE TABLE foo (id INT PRIMARY KEY, count INT)", done); + }); + after(function(done) { + db.wait(done); + }); + + it('should expand the sql with bindings within callbacks for Statement', function() { + db.serialize(() => { + /** test Statement methods */ + let stmt = db.prepare("INSERT INTO foo VALUES(?,?)", function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(NULL,NULL)"); + return this; + }); + /** bind */ + stmt.bind(3,4, function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(3,4)"); + }); + stmt.bind(1,2, function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(1,2)"); + }); + /** reset */ + stmt.reset(function() { + // currently it is being retained + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(1,2)"); + }); + + // only a utility for callbacks + assert.equal(stmt.expandedSql, undefined); + + stmt = db.prepare("INSERT INTO foo VALUES(?,?)", function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(NULL,NULL)"); + return this; + }); + /** run */ + stmt.run([2,2], function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(2,2)"); + }); + stmt.run(4,5, function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(4,5)"); + }); + stmt.finalize(); + + /** get */ + stmt = db.prepare("select * from foo where id = $id;"); + stmt.get(function() { + assert.equal(this.expandedSql, "select * from foo where id = NULL;"); + }); + stmt.get({ $id: 1 }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 1;"); + }); + stmt.get([2], function() { + assert.equal(this.expandedSql, "select * from foo where id = 2;"); + }); + + /** all */ + stmt.all(1, function() { + assert.equal(this.expandedSql, "select * from foo where id = 1;"); + }); + stmt.all({ $id: 2 }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 2;"); + }); + stmt.all([3], function() { + assert.equal(this.expandedSql, "select * from foo where id = 3;"); + }); + + /** each */ + stmt.each(1, function() { + assert.equal(this.expandedSql, "select * from foo where id = 1;"); + }); + stmt.each({ $id: 2 }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 2;"); + }); + stmt.each([3], function() { + assert.equal(this.expandedSql, "select * from foo where id = 3;"); + }); + + stmt.finalize(); + }); + }); + + it('should expand the sql with bindings within callbacks for Database', function() { + db.serialize(() => { + db.prepare("INSERT INTO foo VALUES(?,?)", function() { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(NULL,NULL)"); + }); + + /** run */ + db.run("INSERT INTO foo VALUES(?,?)", 10, 11, function () { + assert.equal(this.expandedSql, "INSERT INTO foo VALUES(10,11)"); + }); + + /** get */ + db.get("select * from foo;", function() { + assert.equal(this.expandedSql, "select * from foo;"); + }); + db.get("select * from bar;", function() { + // since this is a sqlite error there is nothing to expand afterwards + assert.equal(this.expandedSql, undefined); + }); + + /** all */ + db.all("SELECT * FROM foo;", function(err, rows) { + assert.equal(this.expandedSql, "SELECT * FROM foo;"); + assert.deepStrictEqual(rows.sort((a,b) => a.id - b.id), [{ id: 2, count: 2 }, { id: 4, count: 5}, { id: 10, count: 11 }]); + }); + db.all("SELECT id, count FROM foo WHERE id = ?", 1, function(err, rows) { + assert.equal(this.expandedSql, "SELECT id, count FROM foo WHERE id = 1"); + assert.deepStrictEqual(rows, []); + }); + + /** each */ + db.each("select * from foo;", function() { + assert.equal(this.expandedSql, "select * from foo;"); + }); + db.each("select * from bar;", function() { + // since this is a sqlite error there is nothing to expand afterwards + assert.equal(this.expandedSql, undefined); + }); + + /** exec */ + db.exec("select * from foo where id = ?", function() { + // not applicable here so it is never set + assert.equal(this.expandedSql, undefined); + }); + }); + }); +}); From 7b68a3481e7c23340eb63671746449ede1caef2f Mon Sep 17 00:00:00 2001 From: smith-xyz <51304449+smith-xyz@users.noreply.github.com> Date: Sun, 12 Feb 2023 14:18:52 -0500 Subject: [PATCH 2/5] fix: unused typing for Database prepare --- lib/sqlite3.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sqlite3.d.ts b/lib/sqlite3.d.ts index bc2e4a89d..4797c57b8 100644 --- a/lib/sqlite3.d.ts +++ b/lib/sqlite3.d.ts @@ -119,7 +119,7 @@ export class Database extends events.EventEmitter { exec(sql: string, callback?: (this: Statement, err: Error | null) => void): this; - prepare(sql: string, callback?: (this: Statement, err: Error | null, expandedSql: string) => void): Statement; + prepare(sql: string, callback?: (this: Statement, err: Error | null) => void): Statement; prepare(sql: string, params: any, callback?: (this: Statement, err: Error | null) => void): Statement; prepare(sql: string, ...params: any[]): Statement; From a73d58008e579e15707ca7fd3f3c0d9f561559a6 Mon Sep 17 00:00:00 2001 From: smith-xyz <51304449+smith-xyz@users.noreply.github.com> Date: Mon, 27 Mar 2023 17:56:21 -0400 Subject: [PATCH 3/5] fix: remove unneeded call --- src/statement.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/statement.cc b/src/statement.cc index 03e7403c0..ff2a3a378 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -801,8 +801,6 @@ void Statement::Work_AfterReset(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); - // Fire callbacks. Napi::Function cb = baton->callback.Value(); if (IS_FUNCTION(cb)) { From b2622f9d99e7a3c3077b942494df91236e09020c Mon Sep 17 00:00:00 2001 From: smith-xyz Date: Sat, 1 Apr 2023 11:23:31 -0400 Subject: [PATCH 4/5] fix: expanding sql async for each method --- src/macros.h | 5 ++--- src/statement.cc | 13 ++++++------- test/expandedSql.test.js | 10 +++++++++- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/macros.h b/src/macros.h index 4e3ccfa9e..55af6ca8b 100644 --- a/src/macros.h +++ b/src/macros.h @@ -165,12 +165,11 @@ inline bool OtherIsInt(Napi::Number source) { } \ sqlite3_mutex* name = sqlite3_db_mutex(stmt->db->_handle); -#define STATEMENT_EXPAND_SQL() \ +#define STATEMENT_EXPAND_SQL(stmt) \ if (stmt->_handle != NULL) { \ Napi::Env env = stmt->Env(); \ Napi::Value expanded_sql = Napi::String::New(env, sqlite3_expanded_sql(stmt->_handle)); \ - Napi::Value key = Napi::String::New(env, "expandedSql"); \ - (stmt->Value()).Set(key, expanded_sql); \ + (stmt->Value()).Set(Napi::String::New(env, "expandedSql"), expanded_sql); \ } #define STATEMENT_END() \ diff --git a/src/statement.cc b/src/statement.cc index ff2a3a378..9a3acaf5a 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -161,7 +161,7 @@ void Statement::Work_AfterPrepare(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); + STATEMENT_EXPAND_SQL(stmt); if (stmt->status != SQLITE_OK) { Error(baton.get()); @@ -373,7 +373,7 @@ void Statement::Work_AfterBind(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); + STATEMENT_EXPAND_SQL(stmt); if (stmt->status != SQLITE_OK) { Error(baton.get()); @@ -442,7 +442,7 @@ void Statement::Work_AfterGet(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); + STATEMENT_EXPAND_SQL(stmt); if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { Error(baton.get()); @@ -518,7 +518,7 @@ void Statement::Work_AfterRun(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); + STATEMENT_EXPAND_SQL(stmt); if (stmt->status != SQLITE_ROW && stmt->status != SQLITE_DONE) { Error(baton.get()); @@ -589,7 +589,7 @@ void Statement::Work_AfterAll(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); + STATEMENT_EXPAND_SQL(stmt); if (stmt->status != SQLITE_DONE) { Error(baton.get()); @@ -721,6 +721,7 @@ void Statement::AsyncEach(uv_async_t* handle) { NODE_SQLITE3_MUTEX_LOCK(&async->mutex) rows.swap(async->data); NODE_SQLITE3_MUTEX_UNLOCK(&async->mutex) + STATEMENT_EXPAND_SQL(async->stmt); if (rows.empty()) { break; @@ -763,8 +764,6 @@ void Statement::Work_AfterEach(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); - STATEMENT_EXPAND_SQL(); - if (stmt->status != SQLITE_DONE) { Error(baton.get()); } diff --git a/test/expandedSql.test.js b/test/expandedSql.test.js index 29691b906..46ac4adf1 100644 --- a/test/expandedSql.test.js +++ b/test/expandedSql.test.js @@ -70,15 +70,21 @@ describe('expandedSql', function() { assert.equal(this.expandedSql, "select * from foo where id = 3;"); }); - /** each */ + /** each - testing within each callback */ stmt.each(1, function() { assert.equal(this.expandedSql, "select * from foo where id = 1;"); + }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 1;"); }); stmt.each({ $id: 2 }, function() { assert.equal(this.expandedSql, "select * from foo where id = 2;"); + }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 2;"); }); stmt.each([3], function() { assert.equal(this.expandedSql, "select * from foo where id = 3;"); + }, function() { + assert.equal(this.expandedSql, "select * from foo where id = 3;"); }); stmt.finalize(); @@ -118,6 +124,8 @@ describe('expandedSql', function() { /** each */ db.each("select * from foo;", function() { assert.equal(this.expandedSql, "select * from foo;"); + }, function() { + assert.equal(this.expandedSql, "select * from foo;"); }); db.each("select * from bar;", function() { // since this is a sqlite error there is nothing to expand afterwards From 4c59916164306be99f3ce6899a7b8a080c7f7b19 Mon Sep 17 00:00:00 2001 From: smith-xyz <51304449+smith-xyz@users.noreply.github.com> Date: Mon, 3 Apr 2023 12:22:20 -0400 Subject: [PATCH 5/5] fix: ensure scope expands sql for each method --- src/statement.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/statement.cc b/src/statement.cc index 9a3acaf5a..0f3a3bcaa 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -715,13 +715,14 @@ void Statement::AsyncEach(uv_async_t* handle) { Napi::Env env = async->stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(async->stmt); + while (true) { // Get the contents out of the data cache for us to process in the JS callback. Rows rows; NODE_SQLITE3_MUTEX_LOCK(&async->mutex) rows.swap(async->data); NODE_SQLITE3_MUTEX_UNLOCK(&async->mutex) - STATEMENT_EXPAND_SQL(async->stmt); if (rows.empty()) { break; @@ -764,6 +765,8 @@ void Statement::Work_AfterEach(napi_env e, napi_status status, void* data) { Napi::Env env = stmt->Env(); Napi::HandleScope scope(env); + STATEMENT_EXPAND_SQL(stmt); + if (stmt->status != SQLITE_DONE) { Error(baton.get()); }