Skip to content

Commit 2611aa7

Browse files
committed
fixup: apply review suggestions
1 parent 4299a25 commit 2611aa7

File tree

2 files changed

+50
-74
lines changed

2 files changed

+50
-74
lines changed

src/node_sqlite.cc

Lines changed: 49 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -67,62 +67,52 @@ using v8::Value;
6767
} \
6868
} while (0)
6969

70-
#define SQLITE_VALUE_TO_JS( \
71-
from, unreachable_message, isolate, use_big_int_args, ...) \
72-
switch (sqlite3_##from##_type(__VA_ARGS__)) { \
73-
case SQLITE_INTEGER: { \
74-
sqlite3_int64 val = sqlite3_##from##_int64(__VA_ARGS__); \
75-
if (use_big_int_args) { \
76-
return BigInt::New(isolate, val); \
77-
} else if (std::abs(val) <= kMaxSafeJsInteger) { \
78-
return Number::New(isolate, val); \
79-
} else { \
80-
return MaybeLocal<Value>(); \
70+
#define SQLITE_VALUE_TO_JS(from, isolate, use_big_int_args, result, ...) \
71+
do { \
72+
switch (sqlite3_##from##_type(__VA_ARGS__)) { \
73+
case SQLITE_INTEGER: { \
74+
sqlite3_int64 val = sqlite3_##from##_int64(__VA_ARGS__); \
75+
if (use_big_int_args) { \
76+
result = BigInt::New(isolate, val); \
77+
} else if (std::abs(val) <= kMaxSafeJsInteger) { \
78+
result = Number::New(isolate, val); \
79+
} else { \
80+
THROW_ERR_OUT_OF_RANGE(isolate, \
81+
"Value is too large to be represented as a " \
82+
"JavaScript number: %" PRId64, \
83+
val); \
84+
} \
85+
break; \
8186
} \
82-
break; \
83-
} \
84-
case SQLITE_FLOAT: { \
85-
return Number::New(isolate, sqlite3_##from##_double(__VA_ARGS__)); \
86-
break; \
87-
} \
88-
case SQLITE_TEXT: { \
89-
const char* v = \
90-
reinterpret_cast<const char*>(sqlite3_##from##_text(__VA_ARGS__)); \
91-
return String::NewFromUtf8(isolate, v).As<Value>(); \
92-
} \
93-
case SQLITE_NULL: \
94-
return Null(isolate); \
95-
case SQLITE_BLOB: { \
96-
size_t size = static_cast<size_t>(sqlite3_##from##_bytes(__VA_ARGS__)); \
97-
auto data = reinterpret_cast<const uint8_t*>( \
98-
sqlite3_##from##_blob(__VA_ARGS__)); \
99-
auto store = ArrayBuffer::NewBackingStore(isolate, size); \
100-
memcpy(store->Data(), data, size); \
101-
auto ab = ArrayBuffer::New(isolate, std::move(store)); \
102-
return Uint8Array::New(ab, 0, size); \
87+
case SQLITE_FLOAT: { \
88+
result = Number::New(isolate, sqlite3_##from##_double(__VA_ARGS__)); \
89+
break; \
90+
} \
91+
case SQLITE_TEXT: { \
92+
const char* v = \
93+
reinterpret_cast<const char*>(sqlite3_##from##_text(__VA_ARGS__)); \
94+
result = String::NewFromUtf8(isolate, v).As<Value>(); \
95+
break; \
96+
} \
97+
case SQLITE_NULL: { \
98+
result = Null(isolate); \
99+
break; \
100+
} \
101+
case SQLITE_BLOB: { \
102+
size_t size = \
103+
static_cast<size_t>(sqlite3_##from##_bytes(__VA_ARGS__)); \
104+
auto data = reinterpret_cast<const uint8_t*>( \
105+
sqlite3_##from##_blob(__VA_ARGS__)); \
106+
auto store = ArrayBuffer::NewBackingStore(isolate, size); \
107+
memcpy(store->Data(), data, size); \
108+
auto ab = ArrayBuffer::New(isolate, std::move(store)); \
109+
result = Uint8Array::New(ab, 0, size); \
110+
break; \
111+
} \
112+
default: \
113+
UNREACHABLE("Bad SQLite value"); \
103114
} \
104-
default: \
105-
UNREACHABLE(unreachable_message); \
106-
}
107-
108-
MaybeLocal<Value> SQLiteValueToJS(Isolate* isolate,
109-
bool use_big_int_args,
110-
sqlite3_value* sqlite_value) {
111-
SQLITE_VALUE_TO_JS(
112-
value, "Bad SQLite value", isolate, use_big_int_args, sqlite_value);
113-
}
114-
115-
MaybeLocal<Value> SQLiteValueToJS(Isolate* isolate,
116-
bool use_big_int_args,
117-
sqlite3_stmt* stmt,
118-
int column) {
119-
SQLITE_VALUE_TO_JS(column,
120-
"Bad SQLite column type",
121-
isolate,
122-
use_big_int_args,
123-
stmt,
124-
column);
125-
}
115+
} while (0)
126116

127117
inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate,
128118
const char* message) {
@@ -413,17 +403,12 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
413403

414404
for (int i = 0; i < argc; ++i) {
415405
sqlite3_value* value = argv[i];
416-
MaybeLocal<Value> js_val =
417-
SQLiteValueToJS(isolate, self->use_bigint_args_, value);
418-
406+
MaybeLocal<Value> js_val = MaybeLocal<Value>();
407+
SQLITE_VALUE_TO_JS(value, isolate, self->use_bigint_args_, js_val, value);
419408
if (js_val.IsEmpty()) {
420409
// Ignore the SQLite error because a JavaScript exception is pending.
421410
self->db_->SetIgnoreNextSQLiteError(true);
422411
sqlite3_result_error(ctx, "", 0);
423-
THROW_ERR_OUT_OF_RANGE(isolate,
424-
"Value is too large to be represented as a "
425-
"JavaScript number: %" PRId64,
426-
sqlite3_value_int64(value));
427412
return;
428413
}
429414

@@ -1536,18 +1521,9 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
15361521

15371522
MaybeLocal<Value> StatementSync::ColumnToValue(const int column) {
15381523
Isolate* isolate = env()->isolate();
1539-
MaybeLocal<Value> js_val =
1540-
SQLiteValueToJS(isolate, use_big_ints_, statement_, column);
1541-
1542-
if (js_val.IsEmpty()) {
1543-
THROW_ERR_OUT_OF_RANGE(isolate,
1544-
"The value of column %d is too large to be "
1545-
"represented as a JavaScript number: %" PRId64,
1546-
column,
1547-
sqlite3_column_int64(statement_, column));
1548-
return MaybeLocal<Value>();
1549-
}
1550-
1524+
MaybeLocal<Value> js_val = MaybeLocal<Value>();
1525+
SQLITE_VALUE_TO_JS(
1526+
column, isolate, use_big_ints_, js_val, statement_, column);
15511527
return js_val;
15521528
}
15531529

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
282282
bad.get();
283283
}, {
284284
code: 'ERR_OUT_OF_RANGE',
285-
message: /^The value of column 0 is too large.*: 9007199254740992$/,
285+
message: /^Value is too large to be represented as a JavaScript number: 9007199254740992$/,
286286
});
287287
const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
288288
good.setReadBigInts(true);

0 commit comments

Comments
 (0)