Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c126543

Browse filesBrowse files
tniessenaduh95
tniessen
authored and
aduh95
committed
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. PR-URL: #54721 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 67f5f46 commit c126543
Copy full SHA for c126543

File tree

Expand file treeCollapse file tree

4 files changed

+47
-19
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+47
-19
lines changed
Open diff view settings
Collapse file

‎doc/api/sqlite.md‎

Copy file name to clipboardExpand all lines: doc/api/sqlite.md
+7-7Lines changed: 7 additions & 7 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -194,17 +194,17 @@ objects. If the prepared statement does not return any results, this method
194194
returns an empty array. The prepared statement [parameters are bound][] using
195195
the values in `namedParameters` and `anonymousParameters`.
196196

197-
### `statement.expandedSQL()`
197+
### `statement.expandedSQL`
198198

199199
<!-- YAML
200200
added: v22.5.0
201201
-->
202202

203-
* Returns: {string} The source SQL expanded to include parameter values.
203+
* {string} The source SQL expanded to include parameter values.
204204

205-
This method returns the source SQL of the prepared statement with parameter
205+
The source SQL text of the prepared statement with parameter
206206
placeholders replaced by the values that were used during the most recent
207-
execution of this prepared statement. This method is a wrapper around
207+
execution of this prepared statement. This property is a wrapper around
208208
[`sqlite3_expanded_sql()`][].
209209

210210
### `statement.get([namedParameters][, ...anonymousParameters])`
@@ -293,15 +293,15 @@ be used to read `INTEGER` data using JavaScript `BigInt`s. This method has no
293293
impact on database write operations where numbers and `BigInt`s are both
294294
supported at all times.
295295

296-
### `statement.sourceSQL()`
296+
### `statement.sourceSQL`
297297

298298
<!-- YAML
299299
added: v22.5.0
300300
-->
301301

302-
* Returns: {string} The source SQL used to create this prepared statement.
302+
* {string} The source SQL used to create this prepared statement.
303303

304-
This method returns the source SQL of the prepared statement. This method is a
304+
The source SQL text of the prepared statement. This property is a
305305
wrapper around [`sqlite3_sql()`][].
306306

307307
### Type conversion between JavaScript and SQLite
Collapse file

‎src/node_sqlite.cc‎

Copy file name to clipboardExpand all lines: src/node_sqlite.cc
+31-4Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@ using v8::Array;
1818
using v8::ArrayBuffer;
1919
using v8::BigInt;
2020
using v8::Boolean;
21+
using v8::ConstructorBehavior;
2122
using v8::Context;
23+
using v8::DontDelete;
2224
using v8::Exception;
25+
using v8::FunctionCallback;
2326
using v8::FunctionCallbackInfo;
2427
using v8::FunctionTemplate;
2528
using v8::Integer;
@@ -31,6 +34,7 @@ using v8::Name;
3134
using v8::Null;
3235
using v8::Number;
3336
using v8::Object;
37+
using v8::SideEffectType;
3438
using v8::String;
3539
using v8::Uint8Array;
3640
using v8::Value;
@@ -643,7 +647,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
643647
args.GetReturnValue().Set(result);
644648
}
645649

646-
void StatementSync::SourceSQL(const FunctionCallbackInfo<Value>& args) {
650+
void StatementSync::SourceSQLGetter(const FunctionCallbackInfo<Value>& args) {
647651
StatementSync* stmt;
648652
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
649653
Environment* env = Environment::GetCurrent(args);
@@ -657,7 +661,7 @@ void StatementSync::SourceSQL(const FunctionCallbackInfo<Value>& args) {
657661
args.GetReturnValue().Set(sql);
658662
}
659663

660-
void StatementSync::ExpandedSQL(const FunctionCallbackInfo<Value>& args) {
664+
void StatementSync::ExpandedSQLGetter(const FunctionCallbackInfo<Value>& args) {
661665
StatementSync* stmt;
662666
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
663667
Environment* env = Environment::GetCurrent(args);
@@ -717,6 +721,23 @@ void IllegalConstructor(const FunctionCallbackInfo<Value>& args) {
717721
node::THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args));
718722
}
719723

724+
static inline void SetSideEffectFreeGetter(
725+
Isolate* isolate,
726+
Local<FunctionTemplate> class_template,
727+
Local<String> name,
728+
FunctionCallback fn) {
729+
Local<FunctionTemplate> getter =
730+
FunctionTemplate::New(isolate,
731+
fn,
732+
Local<Value>(),
733+
v8::Signature::New(isolate, class_template),
734+
/* length */ 0,
735+
ConstructorBehavior::kThrow,
736+
SideEffectType::kHasNoSideEffect);
737+
class_template->InstanceTemplate()->SetAccessorProperty(
738+
name, getter, Local<FunctionTemplate>(), DontDelete);
739+
}
740+
720741
Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
721742
Environment* env) {
722743
Local<FunctionTemplate> tmpl =
@@ -730,8 +751,14 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
730751
SetProtoMethod(isolate, tmpl, "all", StatementSync::All);
731752
SetProtoMethod(isolate, tmpl, "get", StatementSync::Get);
732753
SetProtoMethod(isolate, tmpl, "run", StatementSync::Run);
733-
SetProtoMethod(isolate, tmpl, "sourceSQL", StatementSync::SourceSQL);
734-
SetProtoMethod(isolate, tmpl, "expandedSQL", StatementSync::ExpandedSQL);
754+
SetSideEffectFreeGetter(isolate,
755+
tmpl,
756+
FIXED_ONE_BYTE_STRING(isolate, "sourceSQL"),
757+
StatementSync::SourceSQLGetter);
758+
SetSideEffectFreeGetter(isolate,
759+
tmpl,
760+
FIXED_ONE_BYTE_STRING(isolate, "expandedSQL"),
761+
StatementSync::ExpandedSQLGetter);
735762
SetProtoMethod(isolate,
736763
tmpl,
737764
"setAllowBareNamedParameters",
Collapse file

‎src/node_sqlite.h‎

Copy file name to clipboardExpand all lines: src/node_sqlite.h
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ class StatementSync : public BaseObject {
6262
static void All(const v8::FunctionCallbackInfo<v8::Value>& args);
6363
static void Get(const v8::FunctionCallbackInfo<v8::Value>& args);
6464
static void Run(const v8::FunctionCallbackInfo<v8::Value>& args);
65-
static void SourceSQL(const v8::FunctionCallbackInfo<v8::Value>& args);
66-
static void ExpandedSQL(const v8::FunctionCallbackInfo<v8::Value>& args);
65+
static void SourceSQLGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
66+
static void ExpandedSQLGetter(
67+
const v8::FunctionCallbackInfo<v8::Value>& args);
6768
static void SetAllowBareNamedParameters(
6869
const v8::FunctionCallbackInfo<v8::Value>& args);
6970
static void SetReadBigInts(const v8::FunctionCallbackInfo<v8::Value>& args);
Collapse file

‎test/parallel/test-sqlite-statement-sync.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-statement-sync.js
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ suite('StatementSync.prototype.run()', () => {
135135
});
136136
});
137137

138-
suite('StatementSync.prototype.sourceSQL()', () => {
139-
test('returns input SQL', (t) => {
138+
suite('StatementSync.prototype.sourceSQL', () => {
139+
test('equals input SQL', (t) => {
140140
const db = new DatabaseSync(nextDb());
141141
t.after(() => { db.close(); });
142142
const setup = db.exec(
@@ -145,12 +145,12 @@ suite('StatementSync.prototype.sourceSQL()', () => {
145145
t.assert.strictEqual(setup, undefined);
146146
const sql = 'INSERT INTO types (key, val) VALUES ($k, $v)';
147147
const stmt = db.prepare(sql);
148-
t.assert.strictEqual(stmt.sourceSQL(), sql);
148+
t.assert.strictEqual(stmt.sourceSQL, sql);
149149
});
150150
});
151151

152-
suite('StatementSync.prototype.expandedSQL()', () => {
153-
test('returns expanded SQL', (t) => {
152+
suite('StatementSync.prototype.expandedSQL', () => {
153+
test('equals expanded SQL', (t) => {
154154
const db = new DatabaseSync(nextDb());
155155
t.after(() => { db.close(); });
156156
const setup = db.exec(
@@ -164,7 +164,7 @@ suite('StatementSync.prototype.expandedSQL()', () => {
164164
stmt.run({ $k: '33' }, '42'),
165165
{ changes: 1, lastInsertRowid: 33 },
166166
);
167-
t.assert.strictEqual(stmt.expandedSQL(), expanded);
167+
t.assert.strictEqual(stmt.expandedSQL, expanded);
168168
});
169169
});
170170

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.