Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full support 64 bit integers with BigInt #1501

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

renatoalencar
Copy link

Well this is not full ready yet, but I'd like to bring some discussions:

  1. To use SQLite int64 instead of int32, because JS Number type is always 64 bits;
  2. That would make possible to serialize BigInt in queries;
  3. Also would make possible to use numbers (with BigInt) between 2^53 and -2^53.

@juanrgm
Copy link

juanrgm commented May 4, 2022

This seems fine, why is it locked?

@daniellockyer
Copy link
Member

@renatoalencar Would you be able to rebase the PR? 🙂

@renatoalencar
Copy link
Author

@renatoalencar Would you be able to rebase the PR? 🙂

Sure! I'll try to do that later today.

@daniellockyer
Copy link
Member

@renatoalencar Thanks! I've had a quick look and I think the PR might need some gating on NAPI_VERSION, because I'd like to maintain N-API v3 support for Node 11/13 (preferably we can avoid doing a breaking release right now). But it LGTM otherwise 🙂

@renatoalencar
Copy link
Author

@renatoalencar Thanks! I've had a quick look and I think the PR might need some gating on NAPI_VERSION, because I'd like to maintain N-API v3 support for Node 11/13 (preferably we can avoid doing a breaking release right now). But it LGTM otherwise 🙂

I was expecting something like that, as I've mentioned my first implementation wasn't really ready. But I added the extra checks and seems to be working.

Thank you

@juanrgm
Copy link

juanrgm commented Jun 20, 2022

In the meantime, I forked your repository at https://github.com/juanrgm/node-sqlite3/tree/int64-support and forced the build from source (juanrgm@881d6bf) for installing with npm easily:

npm install https://github.com/juanrgm/node-sqlite3/tarball/int64-support
const sqlite3 = require('sqlite3').verbose();
const db = new sqlite3.Database(':memory:');

db.serialize(() => {
    db.run("CREATE TABLE t (v INTEGER)");
    const v = 8876343627091516928n
    const stmt = db.prepare("INSERT INTO t VALUES (?)");
    stmt.run(v)
    stmt.finalize();
    db.each("SELECT v FROM t", (err, row) => {
        console.log({ row });
        if (row.v !== v)
            throw new Error('Not equal')
    });
});

db.close();

@magicdawn
Copy link

I have issue when using "sqlite3": "juanrgm/node-sqlite3#int64-support"

gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
action_after_build.target.mk:6: *** missing separator.  Stop.
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2

I guess it's related to @mapbox/node-pre-gyp


I have merge this branch with master, and conflict free, and test passes. can this PR continued ?

https://github.com/TryGhost/node-sqlite3/compare/master...magicdawn:node-sqlite3:int64-support-with-main?expand=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants