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

feat: Add Deno.sqlite bindings #14627

Closed
wants to merge 13 commits into from
Closed

Conversation

littledivy
Copy link
Member

@littledivy
Copy link
Member Author

littledivy commented May 16, 2022

This patch:

./target/release/deno run -A --unstable sqlite.js
cpu: Apple M1
runtime: deno 1.21.3 (aarch64-apple-darwin)

benchmark                        time (avg)             (min … max)       p75       p99      p995
------------------------------------------------------------------- -----------------------------
SELECT * From "Order"         17.83 ms/iter   (17.55 ms … 18.31 ms)  17.94 ms  18.31 ms  18.31 ms
SELECT * From "Product"       41.42 µs/iter  (40.79 µs … 160.83 µs)  41.33 µs  44.21 µs  45.83 µs
SELECT * From "OrderDetail"  335.29 ms/iter (310.42 ms … 399.25 ms) 323.45 ms 399.25 ms 399.25 ms

Node better-sqlite:

node ../better-sqlite.mjs
cpu: Apple M1
runtime: node v16.13.1 (arm64-darwin)

benchmark                        time (avg)             (min … max)       p75       p99      p995
------------------------------------------------------------------- -----------------------------
SELECT * From "Order"         25.57 ms/iter   (25.37 ms … 26.09 ms)  25.65 ms  26.09 ms  26.09 ms
SELECT * From "Product"       68.05 µs/iter  (66.42 µs … 371.75 µs)  67.88 µs  73.46 µs  78.46 µs
SELECT * From "OrderDetail"  546.51 ms/iter (515.57 ms … 602.88 ms) 571.54 ms 602.88 ms 602.88 ms

Benchmarks for FFI / WASM modules can be found here

@bartlomieju bartlomieju changed the title experiment(runtime): expose SQLite bindings feat: Add Deno.sqlite bindings Jul 8, 2022
@bartlomieju
Copy link
Member

bartlomieju commented Jul 13, 2022

Discussed during the last CLI design meeting, there's no support for this feature from the team (it should be done as a userland module either in WASM or using FFI API), so we're closing this PR.

@fdionisi
Copy link

That's a real pity. I was looking forward to this. In conjunction with #13122, it would have been a killer feature (at least for my use case 🙃).

@sant123
Copy link

sant123 commented Jul 14, 2022

Discussed during the last CLI design meeting, there's no support for this feature from the team (it should be done as a userland module either in WASM or using FFI API), so we're closing this PR.

@bartlomieju Could this be re-considered? Looking at the commits I was hoping the release to be completed soon and for my surprise got this message.

@lucacasonato
Copy link
Member

Could this be re-considered?

It's was the third time we had reconsidered this feature in 6 months 😅

What is your use-case that requires this to be built in, instead of using a user-land module?

@mikrosystheme
Copy link

This is very unfortunate. SQLite is a fundamental piece of technology that is useful in almost every software. @lucacasonato Instead of asking for use-cases (there are too many and it is pointless to list them all), what are the reasons to don't have it in core? Using a third party module adds friction (which one? will it be maintained in the long run? it is optimal or a subpar WASM version?) that should be avoided.

@fdionisi
Copy link

@lucacasonato, of course, user-land modules are a viable option, but I agree with @mikrosystheme observations.

Landing SQLite would allow fast prototyping in Deno with a database out of the box; if/when the prototype grows, you then can change the db with a more scalable solution. Same idea behind including TypeScript in the runtime (JS -> TS).
Then, with Jupyter integration, it would help answer the question "why not Python?", which is always hard to answer.

But then again, I suppose it is not sustainable to add everything shiny into Deno. 😅 It is just that seeing this PR growing got me excited about the possibilities that Deno could provide with just a single executable.

To answer your question, I have no solid use case that third-party modules cannot solve. But it would be a tangible improvement to DX, and more solid ground to push for Deno adoption.

@sant123
Copy link

sant123 commented Jul 14, 2022

Could this be re-considered?

It's was the third time we had reconsidered this feature in 6 months 😅

What is your use-case that requires this to be built in, instead of using a user-land module?

Well basically the out of the box feature is something that most people will like. I know there are third party modules that would solve the issue but they will not be as performant as a native built in integration. Also there is a library that uses SQLite bindings using FFI that I think is pretty cool but unfortunately cannot be used in Deno deploy. So having the SQLite built in Deno would be useful for both fast prototyping and secure ACID compliant operations.

@sant123
Copy link

sant123 commented Jul 30, 2022

@bartlomieju @lucacasonato could you consider it again please? 😅🙏🏼 I apologize for being such insistent.

@bartlomieju
Copy link
Member

@sant123 I've been and still am supportive of this feature, however there's no broader support from the rest of the team. We might revisit this topic in a few weeks. I suggest to open a new issue or revive an existing one to get more feedback from the community.

@littledivy littledivy mentioned this pull request Aug 1, 2022
@kitsonk
Copy link
Contributor

kitsonk commented Aug 1, 2022

Issue #11657 was re-opened

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

Successfully merging this pull request may close these issues.

7 participants