From e25579c683695dc9074b7373452c660342fb6462 Mon Sep 17 00:00:00 2001 From: Renato Alencar Date: Thu, 23 Sep 2021 16:09:22 -0300 Subject: [PATCH 1/4] Full support 64 bit integers with BigInt --- binding.gyp | 2 +- src/macros.h | 3 +++ src/statement.cc | 25 +++++++++++++++++++++++-- test/other_objects.test.js | 21 ++++++++++++++++++++- 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/binding.gyp b/binding.gyp index f1336f6f7..f2fdebbe8 100644 --- a/binding.gyp +++ b/binding.gyp @@ -49,7 +49,7 @@ "src/node_sqlite3.cc", "src/statement.cc" ], - "defines": [ "NAPI_VERSION=<(napi_build_version)", "NAPI_DISABLE_CPP_EXCEPTIONS=1" ] + "defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS=1" ] }, { "target_name": "action_after_build", diff --git a/src/macros.h b/src/macros.h index 1c1a7c445..fd21feb00 100644 --- a/src/macros.h +++ b/src/macros.h @@ -23,6 +23,9 @@ inline bool OtherIsInt(Napi::Number source) { } } +#define JS_MAX_SAFE_INTEGER 9007199254740991 +#define JS_MIN_SAFE_INTEGER -JS_MAX_SAFE_INTEGER + #define REQUIRE_ARGUMENTS(n) \ if (info.Length() < (n)) { \ Napi::TypeError::New(env, "Expected " #n "arguments").ThrowAsJavaScriptException(); \ diff --git a/src/statement.cc b/src/statement.cc index 1981df954..0c5eb804c 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -204,6 +204,20 @@ template Values::Field* else if (OtherInstanceOf(source.As(), "Date")) { return new Values::Float(pos, source.ToNumber().DoubleValue()); } + else if (source.IsBigInt()) { + bool hadLoss; + auto ret = new Values::Integer(pos, source.As().Int64Value(&hadLoss)); + + if (!hadLoss) { + Napi::TypeError::New( + source.Env(), + "Converting BigInt to internal representation results is not lossless.") + .ThrowAsJavaScriptException(); + return NULL; + } + + return ret; + } else if (source.IsObject()) { Napi::String napiVal = source.ToString(); // Check whether toString returned a value that is not undefined. @@ -299,7 +313,7 @@ bool Statement::Bind(const Parameters & parameters) { switch (field->type) { case SQLITE_INTEGER: { - status = sqlite3_bind_int(_handle, pos, + status = sqlite3_bind_int64(_handle, pos, ((Values::Integer*)field)->value); } break; case SQLITE_FLOAT: { @@ -810,7 +824,14 @@ Napi::Value Statement::RowToJS(Napi::Env env, Row* row) { switch (field->type) { case SQLITE_INTEGER: { - value = Napi::Number::New(env, ((Values::Integer*)field)->value); + auto field_value = ((Values::Integer*)field)->value; + + if (field_value > JS_MAX_SAFE_INTEGER || field_value < JS_MIN_SAFE_INTEGER) { + value = Napi::BigInt::New(env, field_value); + } + else { + value = Napi::Number::New(env, ((Values::Integer*)field)->value); + } } break; case SQLITE_FLOAT: { value = Napi::Number::New(env, ((Values::Float*)field)->value); diff --git a/test/other_objects.test.js b/test/other_objects.test.js index 718598768..e57289ae6 100644 --- a/test/other_objects.test.js +++ b/test/other_objects.test.js @@ -8,11 +8,12 @@ describe('data types', function() { db.run("CREATE TABLE txt_table (txt TEXT)"); db.run("CREATE TABLE int_table (int INTEGER)"); db.run("CREATE TABLE flt_table (flt FLOAT)"); + db.run("CREATE TABLE bigint_table (big BIGINT)"); db.wait(done); }); beforeEach(function(done) { - db.exec('DELETE FROM txt_table; DELETE FROM int_table; DELETE FROM flt_table;', done); + db.exec('DELETE FROM txt_table; DELETE FROM int_table; DELETE FROM flt_table; DELETE FROM bigint_table;', done); }); it('should serialize Date()', function(done) { @@ -95,4 +96,22 @@ describe('data types', function() { }); }); + [ + 1n, + 1337n, + 8876343627091516928n, + -8876343627091516928n, + ].forEach(function (bigint) { + it('should serialize BigInt ' + bigint, function (done) { + db.run("INSERT INTO bigint_table VALUES(?)", bigint, function(err) { + if (err) throw err; + db.get("SELECT big AS bigint FROM bigint_table", function (err, row) { + if (err) throw err + assert.equal(row.bigint, bigint); + done(); + }); + }); + }); + }); + }); From 1cf4a9ffa3ec4d6b64c5d08c35bdfa3706a332b1 Mon Sep 17 00:00:00 2001 From: Renato Alencar Date: Fri, 24 Sep 2021 06:25:31 -0300 Subject: [PATCH 2/4] Fix config, set minimum NAPI_VERSION to 6 --- binding.gyp | 2 +- package.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/binding.gyp b/binding.gyp index f2fdebbe8..f1336f6f7 100644 --- a/binding.gyp +++ b/binding.gyp @@ -49,7 +49,7 @@ "src/node_sqlite3.cc", "src/statement.cc" ], - "defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS=1" ] + "defines": [ "NAPI_VERSION=<(napi_build_version)", "NAPI_DISABLE_CPP_EXCEPTIONS=1" ] }, { "target_name": "action_after_build", diff --git a/package.json b/package.json index 03c5ef14c..78d8c59e6 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,6 @@ "remote_path": "v{version}", "package_name": "napi-v{napi_build_version}-{platform}-{libc}-{arch}.tar.gz", "napi_versions": [ - 3, 6 ] }, From 9fae592ad2c44e319a6fb0cc9c7a4fcc6cc88e1a Mon Sep 17 00:00:00 2001 From: Renato Alencar Date: Fri, 24 Sep 2021 06:40:51 -0300 Subject: [PATCH 3/4] Add a test case to cover out of range integer error --- src/statement.cc | 10 +++++----- test/other_objects.test.js | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/statement.cc b/src/statement.cc index 0c5eb804c..939dd8b41 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -205,13 +205,13 @@ template Values::Field* return new Values::Float(pos, source.ToNumber().DoubleValue()); } else if (source.IsBigInt()) { - bool hadLoss; - auto ret = new Values::Integer(pos, source.As().Int64Value(&hadLoss)); + bool lossless; + auto ret = new Values::Integer(pos, source.As().Int64Value(&lossless)); - if (!hadLoss) { - Napi::TypeError::New( + if (!lossless) { + Napi::RangeError::New( source.Env(), - "Converting BigInt to internal representation results is not lossless.") + "Value can't be losslessly converted to 64 bit integer.") .ThrowAsJavaScriptException(); return NULL; } diff --git a/test/other_objects.test.js b/test/other_objects.test.js index e57289ae6..da1232b0f 100644 --- a/test/other_objects.test.js +++ b/test/other_objects.test.js @@ -114,4 +114,18 @@ describe('data types', function() { }); }); + it('should fail to serialize larger numbers', function (done) { + const bigint = 0xffffffffffffffffffffn; // 80 bits + let error; + + try { + db.run('INSERT INTO bigint_table VALUES(?)', bigint); + } catch (err) { + error = err; + } finally { + assert.notEqual(error, undefined); + done(); + } + }) + }); From 730ab55ed06327e489d72daf85a5ea5327befb83 Mon Sep 17 00:00:00 2001 From: Renato Alencar Date: Thu, 12 May 2022 11:44:48 -0300 Subject: [PATCH 4/4] Don't break support for NAPI_VERSION < 6 --- package.json | 1 + src/statement.cc | 10 +++++-- test/other_objects.test.js | 58 ++++++++++++++++++++------------------ 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index 78d8c59e6..03c5ef14c 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "remote_path": "v{version}", "package_name": "napi-v{napi_build_version}-{platform}-{libc}-{arch}.tar.gz", "napi_versions": [ + 3, 6 ] }, diff --git a/src/statement.cc b/src/statement.cc index 939dd8b41..16380b423 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -204,6 +204,7 @@ template Values::Field* else if (OtherInstanceOf(source.As(), "Date")) { return new Values::Float(pos, source.ToNumber().DoubleValue()); } +#if NAPI_VERSION >= 6 else if (source.IsBigInt()) { bool lossless; auto ret = new Values::Integer(pos, source.As().Int64Value(&lossless)); @@ -218,6 +219,7 @@ template Values::Field* return ret; } +#endif else if (source.IsObject()) { Napi::String napiVal = source.ToString(); // Check whether toString returned a value that is not undefined. @@ -826,11 +828,13 @@ Napi::Value Statement::RowToJS(Napi::Env env, Row* row) { case SQLITE_INTEGER: { auto field_value = ((Values::Integer*)field)->value; +#if NAPI_VERSION >= 6 if (field_value > JS_MAX_SAFE_INTEGER || field_value < JS_MIN_SAFE_INTEGER) { value = Napi::BigInt::New(env, field_value); - } - else { - value = Napi::Number::New(env, ((Values::Integer*)field)->value); + } else +#endif + { + value = Napi::Number::New(env, field_value); } } break; case SQLITE_FLOAT: { diff --git a/test/other_objects.test.js b/test/other_objects.test.js index da1232b0f..09c20f56b 100644 --- a/test/other_objects.test.js +++ b/test/other_objects.test.js @@ -96,36 +96,40 @@ describe('data types', function() { }); }); - [ - 1n, - 1337n, - 8876343627091516928n, - -8876343627091516928n, - ].forEach(function (bigint) { - it('should serialize BigInt ' + bigint, function (done) { - db.run("INSERT INTO bigint_table VALUES(?)", bigint, function(err) { - if (err) throw err; - db.get("SELECT big AS bigint FROM bigint_table", function (err, row) { - if (err) throw err - assert.equal(row.bigint, bigint); - done(); + if (process.versions.napi >= '6') { + [ + 1n, + 1337n, + 8876343627091516928n, + -8876343627091516928n, + ].forEach(function (bigint) { + it('should serialize BigInt ' + bigint, function (done) { + db.run("INSERT INTO bigint_table VALUES(?)", bigint, function(err) { + if (err) throw err; + db.get("SELECT big AS bigint FROM bigint_table", function (err, row) { + if (err) throw err + assert.equal(row.bigint, bigint); + done(); + }); }); }); }); - }); - - it('should fail to serialize larger numbers', function (done) { - const bigint = 0xffffffffffffffffffffn; // 80 bits - let error; + } - try { - db.run('INSERT INTO bigint_table VALUES(?)', bigint); - } catch (err) { - error = err; - } finally { - assert.notEqual(error, undefined); - done(); - } - }) + if (process.versions.napi >= '6') { + it('should fail to serialize larger numbers', function (done) { + const bigint = 0xffffffffffffffffffffn; // 80 bits + let error; + + try { + db.run('INSERT INTO bigint_table VALUES(?)', bigint); + } catch (err) { + error = err; + } finally { + assert.notEqual(error, undefined); + done(); + } + }) + } });