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

Use better-sqlite3 #11400

Open
sushantdhiman opened this issue Sep 7, 2019 · 28 comments
Open

Use better-sqlite3 #11400

sushantdhiman opened this issue Sep 7, 2019 · 28 comments
Labels
dialect: new For issues and PRs that relate to new dialect support dialect: sqlite For issues and PRs. Things that involve SQLite (and do not involve all dialects).

Comments

@sushantdhiman
Copy link
Contributor

Thinking about using better-sqlite3 for SQLite database, mainly for supporting 64bit Integers. Their benchmark looks great as well

WiseLibs/better-sqlite3#262

@sushantdhiman sushantdhiman added status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. type: refactor For issues and PRs. Things that improve the code readability, maintainability, testability, etc. dialect: sqlite For issues and PRs. Things that involve SQLite (and do not involve all dialects). breaking change For issues and PRs. Changes that break compatibility and require a major version increment. labels Sep 7, 2019
@sushantdhiman sushantdhiman added this to the v6 milestone Sep 7, 2019
@sushantdhiman sushantdhiman pinned this issue Sep 7, 2019
@papb
Copy link
Member

papb commented Sep 7, 2019

Completely agreed!!

@usebaz
Copy link

usebaz commented Oct 15, 2019

@sushantdhiman Hi, if need help I can make PR with migrate to better-sqlite3

@sushantdhiman
Copy link
Contributor Author

@usebaz Thanks. Soon master branch will point to the next beta. After that you can submit your PR

@etylermoss
Copy link

Is there any update on this?

@sushantdhiman
Copy link
Contributor Author

@etylermoss Currently no one is working on this. I tried to convert some time ago but found that API is really annoying :)

For example,

  1. run, (only on statements that do not return data)
  2. get (only on statements that return data)

This mean as an ORM we need to check what kind of query is going to execute (will it return data?) and switch methods based on that. I think this is unnecessary complication.

I wish they could just provide simple method to execute any arbitrary query (with parameters) like every other dialect library in existence.

So officially we want to switch but we are not working on it. If someone want to do a PR for this, we will welcome that change.

@louy2
Copy link
Contributor

louy2 commented May 18, 2020

How is it handled with node-sqlite3 currently? It has the same run/get split.

@sushantdhiman sushantdhiman changed the title V6: Use better-sqlite3 Use better-sqlite3 Jun 24, 2020
@sushantdhiman sushantdhiman removed this from the v6 milestone Jun 24, 2020
@sushantdhiman sushantdhiman unpinned this issue Jun 25, 2020
@meshakeeb

This comment was marked as spam.

@lesion
Copy link
Contributor

lesion commented Oct 30, 2021

@etylermoss Currently no one is working on this. I tried to convert some time ago but found that API is really annoying :)
For example,
1. run, (only on statements that do not return data)

2. [`get`](https://github.com/JoshuaWise/better-sqlite3/blob/master/docs/api.md#getbindparameters---row) `(only on statements that return data)`

This mean as an ORM we need to check what kind of query is going to execute (will it return data?) and switch methods based on that. I think this is unnecessary complication.

I wish they could just provide simple method to execute any arbitrary query (with parameters) like every other dialect library in existence.

So officially we want to switch but we are not working on it. If someone want to do a PR for this, we will welcome that change.

Hi there!
Is your attempt available somewhere? I'm thinking of trying to implement this

@github-actions

This comment was marked as outdated.

@github-actions github-actions bot added the stale label Nov 7, 2021
@bradisbell
Copy link

Not stale.

@John-H-Smith
Copy link

Any Updates?

@ephys
Copy link
Member

ephys commented Jan 4, 2022

@John-H-Smith we're thinking about doing it for v7 sequelize/meetings#7

@cirosantilli
Copy link

cirosantilli commented Jan 6, 2022

I reproduce their 10x faster benchmarks in my project when migrating from better-sqlite3 to sequelize 6, It is amazing!!

Make sure to compile with that log limiting options maxed out if possible to prevent log truncation: WiseLibs/better-sqlite3#306

@tankakatan
Copy link

Hi everyone,

Please consider adding support for better-sqlite3 package ASAP. We are experiencing lots of issues with node-sqlite3 when shipping our Node.js app as a packaged executable for different platforms. Even though it is possible to include it in the bundle, our users keep reporting very weird and hard to debug issues with the old lib.

@ephys
Copy link
Member

ephys commented Jul 1, 2022

At this moment none of the maintainers are working on replacing sqlite3 with better-sqlite3 and won't be in the foreseeable future since there are many other features that we want to add, and bugs we need to fix. If somebody from the community makes a Pull Request we are of course happy to review that.

@tankakatan
Copy link

tankakatan commented Jul 1, 2022

Thank you very much for the quick response @ephys. In our team we'll estimate the effort needed for the integration, and find out whether we are able to allocate resources for that. Also pinging @usebaz who offered their help with the integration a few years ago. Just in hope they are still available.

@fzn0x
Copy link
Member

fzn0x commented Oct 23, 2022

We may need to take a note about this:

@luka2i
Copy link

luka2i commented Jul 14, 2023

any updates?

@WikiRik
Copy link
Member

WikiRik commented Jul 15, 2023

No updates. We've moved this internally to v8 when we split the various dialects off to separate packages. That way we can even look into supporting multiple connectors for sqlite

@dixiekong
Copy link

any updates?

@j-refs
Copy link

j-refs commented Jan 30, 2024

Is this still not being worked on? If the maintainers approve, I'll gladly open a fork open to the community to get this feature up and running and PR it.

After a quick scan of the source, it doesn't really seem as hard as every is making it, seems like the regular sqlite dialect can be easily modified for better-sqlite3, but I just don't have the time to scan and port everything, maybe if we make it a community effort, we can get this done

@WikiRik
Copy link
Member

WikiRik commented Jan 30, 2024

@j-refs internally this has been postponed to v8 when we split dialects. If you can make it so that users can select which of the two adapters they want to use, we can reconsider adding this to v7.

If you create a fork, please mention it here in this issue as well so we can refer people to it in the meantime

@j-refs
Copy link

j-refs commented Jan 30, 2024

Will look into forsure. Probably later today, I work nights and I'm ready to KO. Any info the maintainers wanna add in before I form?

@yisar
Copy link

yisar commented Feb 23, 2024

any updates?

@j-refs
Copy link

j-refs commented Feb 25, 2024

Yes sorry, struggling to find time to get started on this, but is definitely on my queue

@j-refs
Copy link

j-refs commented Mar 4, 2024

I'm ready to start working on this. Would y'all prefer a branched PR or it's own repo?

@ephys
Copy link
Member

ephys commented Apr 8, 2024

@j-refs So sorry, I did not see your latest message

The SQLite package has been split to its own package (https://github.com/sequelize/sequelize/tree/main/packages). You can fork the repository, create a new package called better-sqlite3, and open a PR to this repository :)

As we're opting for providing both @sequelize/sqlite3 and @sequelize/better-sqlite3, this won't be a breaking change after all

Note that @sequelize/better-sqlite3 cannot extend @sequelize/sqlite3 directly to avoid installing both sqlite3 and better-sqlite3. Instead, anything that you wish to re-use from @sequelize/sqlite3 should be moved to an internal package, @sequelize/abstract-sqlite

@ephys ephys added dialect: new For issues and PRs that relate to new dialect support and removed status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. breaking change For issues and PRs. Changes that break compatibility and require a major version increment. type: refactor For issues and PRs. Things that improve the code readability, maintainability, testability, etc. labels Apr 8, 2024
@0xslipk
Copy link

0xslipk commented Dec 19, 2024

@ephys I will start working on this. Do you guys have something already in progress that can work or help with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: new For issues and PRs that relate to new dialect support dialect: sqlite For issues and PRs. Things that involve SQLite (and do not involve all dialects).
Projects
None yet
Development

No branches or pull requests