Skip to content

Commit

Permalink
sqlite: make sourceSQL and expandedSQL string-valued properties
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
tniessen authored and aduh95 committed Oct 9, 2024
1 parent 67f5f46 commit c126543
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 19 deletions.
14 changes: 7 additions & 7 deletions doc/api/sqlite.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,17 +194,17 @@ 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`

<!-- YAML
added: v22.5.0
-->

* 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
The source SQL text of the prepared statement with parameter
placeholders replaced by the values that were used during the most recent
execution of this prepared statement. This method is a wrapper around
execution of this prepared statement. This property is a wrapper around
[`sqlite3_expanded_sql()`][].

### `statement.get([namedParameters][, ...anonymousParameters])`
Expand Down Expand Up @@ -293,15 +293,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`

<!-- YAML
added: v22.5.0
-->

* 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
Expand Down
35 changes: 31 additions & 4 deletions src/node_sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -643,7 +647,7 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(result);
}

void StatementSync::SourceSQL(const FunctionCallbackInfo<Value>& args) {
void StatementSync::SourceSQLGetter(const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Environment* env = Environment::GetCurrent(args);
Expand All @@ -657,7 +661,7 @@ void StatementSync::SourceSQL(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(sql);
}

void StatementSync::ExpandedSQL(const FunctionCallbackInfo<Value>& args) {
void StatementSync::ExpandedSQLGetter(const FunctionCallbackInfo<Value>& args) {
StatementSync* stmt;
ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -717,6 +721,23 @@ void IllegalConstructor(const FunctionCallbackInfo<Value>& args) {
node::THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args));
}

static inline void SetSideEffectFreeGetter(
Isolate* isolate,
Local<FunctionTemplate> class_template,
Local<String> name,
FunctionCallback fn) {
Local<FunctionTemplate> getter =
FunctionTemplate::New(isolate,
fn,
Local<Value>(),
v8::Signature::New(isolate, class_template),
/* length */ 0,
ConstructorBehavior::kThrow,
SideEffectType::kHasNoSideEffect);
class_template->InstanceTemplate()->SetAccessorProperty(
name, getter, Local<FunctionTemplate>(), DontDelete);
}

Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
Environment* env) {
Local<FunctionTemplate> tmpl =
Expand All @@ -730,8 +751,14 @@ Local<FunctionTemplate> 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",
Expand Down
5 changes: 3 additions & 2 deletions src/node_sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ class StatementSync : public BaseObject {
static void All(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Get(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Run(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SourceSQL(const v8::FunctionCallbackInfo<v8::Value>& args);
static void ExpandedSQL(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SourceSQLGetter(const v8::FunctionCallbackInfo<v8::Value>& args);
static void ExpandedSQLGetter(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetAllowBareNamedParameters(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetReadBigInts(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
12 changes: 6 additions & 6 deletions test/parallel/test-sqlite-statement-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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);
});
});

Expand Down

0 comments on commit c126543

Please sign in to comment.