From 97a80957f410e0c4b44d6592ed9cda5e39997787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 1 Sep 2024 22:14:59 +0000 Subject: [PATCH] sqlite: make sourceSQL and expandedSQL string-valued properties Change sourceSQL and expandedSQL from being methods to being string-valued properties. These fields - are conceptually properties (and not actions), - are derived deterministically from the current state of the object, - require no parameters, and - are inexpensive to compute. Also, following the naming conventions of ECMAScript for new features, most function names should usually contain a verb, whereas names of (dynamically computed) properties generally should not, so the current names also seem more appropriate for properties than for functions. --- doc/api/sqlite.md | 14 ++++----- src/node_sqlite.cc | 35 ++++++++++++++++++--- src/node_sqlite.h | 5 +-- test/parallel/test-sqlite-statement-sync.js | 12 +++---- 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index ac17ecca640d21..985cf2de88beb5 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -189,16 +189,16 @@ objects. If the prepared statement does not return any results, this method returns an empty array. The prepared statement [parameters are bound][] using the values in `namedParameters` and `anonymousParameters`. -### `statement.expandedSQL()` +### `statement.expandedSQL` -* Returns: {string} The source SQL expanded to include parameter values. +* {string} The source SQL expanded to include parameter values. -This method returns the source SQL of the prepared statement with parameter -placeholders replaced by values. This method is a wrapper around +The source SQL text of the prepared statement with parameter +placeholders replaced by values. This property is a wrapper around [`sqlite3_expanded_sql()`][]. ### `statement.get([namedParameters][, ...anonymousParameters])` @@ -287,15 +287,15 @@ be used to read `INTEGER` data using JavaScript `BigInt`s. This method has no impact on database write operations where numbers and `BigInt`s are both supported at all times. -### `statement.sourceSQL()` +### `statement.sourceSQL` -* Returns: {string} The source SQL used to create this prepared statement. +* {string} The source SQL used to create this prepared statement. -This method returns the source SQL of the prepared statement. This method is a +The source SQL text of the prepared statement. This property is a wrapper around [`sqlite3_sql()`][]. ### Type conversion between JavaScript and SQLite diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 14b8263310279a..783011ff4858b8 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -18,8 +18,11 @@ using v8::Array; using v8::ArrayBuffer; using v8::BigInt; using v8::Boolean; +using v8::ConstructorBehavior; using v8::Context; +using v8::DontDelete; using v8::Exception; +using v8::FunctionCallback; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; using v8::Integer; @@ -31,6 +34,7 @@ using v8::Name; using v8::Null; using v8::Number; using v8::Object; +using v8::SideEffectType; using v8::String; using v8::Uint8Array; using v8::Value; @@ -603,7 +607,7 @@ void StatementSync::Run(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } -void StatementSync::SourceSQL(const FunctionCallbackInfo& args) { +void StatementSync::SourceSQLGetter(const FunctionCallbackInfo& args) { StatementSync* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); Environment* env = Environment::GetCurrent(args); @@ -617,7 +621,7 @@ void StatementSync::SourceSQL(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(sql); } -void StatementSync::ExpandedSQL(const FunctionCallbackInfo& args) { +void StatementSync::ExpandedSQLGetter(const FunctionCallbackInfo& args) { StatementSync* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); Environment* env = Environment::GetCurrent(args); @@ -671,6 +675,23 @@ void IllegalConstructor(const FunctionCallbackInfo& args) { node::THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args)); } +static inline void SetSideEffectFreeGetter( + Isolate* isolate, + Local class_template, + Local name, + FunctionCallback fn) { + Local getter = + FunctionTemplate::New(isolate, + fn, + Local(), + v8::Signature::New(isolate, class_template), + /* length */ 0, + ConstructorBehavior::kThrow, + SideEffectType::kHasNoSideEffect); + class_template->InstanceTemplate()->SetAccessorProperty( + name, getter, Local(), DontDelete); +} + Local StatementSync::GetConstructorTemplate( Environment* env) { Local tmpl = @@ -684,8 +705,14 @@ Local StatementSync::GetConstructorTemplate( SetProtoMethod(isolate, tmpl, "all", StatementSync::All); SetProtoMethod(isolate, tmpl, "get", StatementSync::Get); SetProtoMethod(isolate, tmpl, "run", StatementSync::Run); - SetProtoMethod(isolate, tmpl, "sourceSQL", StatementSync::SourceSQL); - SetProtoMethod(isolate, tmpl, "expandedSQL", StatementSync::ExpandedSQL); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "sourceSQL"), + StatementSync::SourceSQLGetter); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "expandedSQL"), + StatementSync::ExpandedSQLGetter); SetProtoMethod(isolate, tmpl, "setAllowBareNamedParameters", diff --git a/src/node_sqlite.h b/src/node_sqlite.h index f62b2e991cb95a..8cb7a2d54619bc 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -60,8 +60,9 @@ class StatementSync : public BaseObject { static void All(const v8::FunctionCallbackInfo& args); static void Get(const v8::FunctionCallbackInfo& args); static void Run(const v8::FunctionCallbackInfo& args); - static void SourceSQL(const v8::FunctionCallbackInfo& args); - static void ExpandedSQL(const v8::FunctionCallbackInfo& args); + static void SourceSQLGetter(const v8::FunctionCallbackInfo& args); + static void ExpandedSQLGetter( + const v8::FunctionCallbackInfo& args); static void SetAllowBareNamedParameters( const v8::FunctionCallbackInfo& args); static void SetReadBigInts(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-sqlite-statement-sync.js b/test/parallel/test-sqlite-statement-sync.js index 1052e9fb76390b..8cd3b64ab4bc66 100644 --- a/test/parallel/test-sqlite-statement-sync.js +++ b/test/parallel/test-sqlite-statement-sync.js @@ -135,8 +135,8 @@ suite('StatementSync.prototype.run()', () => { }); }); -suite('StatementSync.prototype.sourceSQL()', () => { - test('returns input SQL', (t) => { +suite('StatementSync.prototype.sourceSQL', () => { + test('equals input SQL', (t) => { const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = db.exec( @@ -145,12 +145,12 @@ suite('StatementSync.prototype.sourceSQL()', () => { t.assert.strictEqual(setup, undefined); const sql = 'INSERT INTO types (key, val) VALUES ($k, $v)'; const stmt = db.prepare(sql); - t.assert.strictEqual(stmt.sourceSQL(), sql); + t.assert.strictEqual(stmt.sourceSQL, sql); }); }); -suite('StatementSync.prototype.expandedSQL()', () => { - test('returns expanded SQL', (t) => { +suite('StatementSync.prototype.expandedSQL', () => { + test('equals expanded SQL', (t) => { const db = new DatabaseSync(nextDb()); t.after(() => { db.close(); }); const setup = db.exec( @@ -164,7 +164,7 @@ suite('StatementSync.prototype.expandedSQL()', () => { stmt.run({ $k: '33' }, '42'), { changes: 1, lastInsertRowid: 33 }, ); - t.assert.strictEqual(stmt.expandedSQL(), expanded); + t.assert.strictEqual(stmt.expandedSQL, expanded); }); });