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.
  • Loading branch information
tniessen committed Sep 1, 2024
1 parent 2f0b371 commit 97a8095
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 @@ -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`

<!-- 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
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])`
Expand Down Expand Up @@ -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`

<!-- 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 @@ -603,7 +607,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 @@ -617,7 +621,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 @@ -671,6 +675,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 @@ -684,8 +705,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 @@ -60,8 +60,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 97a8095

Please sign in to comment.