sqlite: enable common flags #57621

geeksilva97 · 2025-03-25T17:51:46Z

This PR enables flags that are common to other sqlite players in Node.js ecosystem:

This is related to #56476, even though it does not enable the RBU extension.

I see this as a good step toward the stabilization since it will make it easier for people from other dependencies.

nodejs-github-bot · 2025-03-25T17:51:50Z

Review requested:

@nodejs/gyp
@nodejs/security-wg
@nodejs/sqlite

geeksilva97 · 2025-03-25T17:52:47Z

Not sure whether this will be a problem, but this increases the binary size by 600KB.

cjihrig

Not saying whether or not we should land this, but https://github.com/nodejs/node/blob/main/deps/sqlite/unofficial.gni should be updated, and there should be at least one test for each new API to prevent regressions.

geeksilva97 · 2025-03-25T17:57:11Z

Not saying whether or not we should land this, but https://github.com/nodejs/node/blob/main/deps/sqlite/unofficial.gni should be updated, and there should be at least one test for each new API to prevent regressions.

Good point. Also, I think it would be good to have some benchmarks on sqlite implementations.

geeksilva97 · 2025-03-25T19:23:09Z

Not saying whether or not we should land this, but https://github.com/nodejs/node/blob/main/deps/sqlite/unofficial.gni should be updated, and there should be at least one test for each new API to prevent regressions.

@cjihrig , what's the difference between .gyp and .gni file?

test/parallel/test-sqlite.js

cjihrig · 2025-03-25T19:30:37Z

what's the difference between .gyp and .gni file?

They are for two different build systems. The .gyp file is used by the official build. The .gni file is used by the unofficial GN build.

geeksilva97 · 2025-03-25T19:32:39Z

what's the difference between .gyp and .gni file?

They are for two different build systems. The .gyp file is used by the official build. The .gni file is used by the unofficial GN build.

Thank you!

cjihrig · 2025-03-25T19:42:35Z

even though it does not enable the RBU extension.

Why not? And why not the Geopoly extension?

geeksilva97 · 2025-03-25T20:16:35Z

even though it does not enable the RBU extension.

Why not? And why not the Geopoly extension?

For RBU, I didn't see it in better-sqlite3 or node-sqlite3. Geopoly seems like a good extension to have.

geeksilva97 · 2025-03-25T20:26:06Z

even though it does not enable the RBU extension.

Why not? And why not the Geopoly extension?

For RBU, I didn't see it in better-sqlite3 or node-sqlite3. Geopoly seems like a good extension to have.

On the other hand, the issue mentions the SQLite Amalgamation. I think we could have RBU enabled.

The amalgamation contains everything you need to integrate SQLite into a larger project

codecov · 2025-03-25T21:55:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.23%. Comparing base (1b5b019) to head (c22f75e).
Report is 69 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57621      +/-   ##
==========================================
+ Coverage   90.22%   90.23%   +0.01%     
==========================================
  Files         630      630              
  Lines      185054   185055       +1     
  Branches    36254    36219      -35     
==========================================
+ Hits       166963   166985      +22     
- Misses      11032    11037       +5     
+ Partials     7059     7033      -26

see 45 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

TheOneTheOnlyJJ

Small typo

test/parallel/test-sqlite.js

cjihrig

One comment, but changes LGTM.

test/parallel/test-sqlite.js

Co-authored-by: Colin Ihrig <[email protected]>

TheOneTheOnlyJJ · 2025-03-26T14:05:52Z

test/parallel/test-sqlite.js

+    );
+  });
+
+  test('fts3 parenthesis', (t) => {


This is inconsistent with the rest, missing is enabled in its name.

test/parallel/test-sqlite.js

cjihrig

Still unsure whether or not we want all of these enabled, but giving this a LGTM so it's not stalled.

nodejs-github-bot · 2025-04-03T15:22:02Z

CI: https://ci.nodejs.org/job/node-test-pull-request/66004/

nodejs-github-bot · 2025-04-03T17:45:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/66009/ 🟡

TheOneTheOnlyJJ · 2025-04-03T20:22:06Z

Still unsure whether or not we want all of these enabled, but giving this a LGTM so it's not stalled.

I really hope they land. Not having them enabled limits the usefulness of the library a lot.

bakkot · 2025-04-03T20:28:39Z

I personally don't care about the other ones but I'm really hoping FTS5 specifically lands.

TheOneTheOnlyJJ · 2025-04-03T20:40:04Z

I personally don't care about the other ones but I'm really hoping FTS5 specifically lands.

This is why, for all the various use cases various projects have, they should all be included. 😊

cjihrig · 2025-04-04T16:46:23Z

Just noting that this is ready to land. It just needs one person who believes in this change strongly enough to put the commit-queue label on it.

geeksilva97 · 2025-04-04T16:48:29Z

Let's do this!

nodejs-github-bot · 2025-04-04T16:57:45Z

Landed in 9aa57bf

nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Mar 25, 2025

geeksilva97 force-pushed the enable-common-sqlite-flags branch from e4b93b9 to 9590dee Compare March 25, 2025 17:53

cjihrig reviewed Mar 25, 2025

View reviewed changes

geeksilva97 force-pushed the enable-common-sqlite-flags branch from 9590dee to 0c1ec86 Compare March 25, 2025 19:22

geeksilva97 commented Mar 25, 2025

View reviewed changes

test/parallel/test-sqlite.js Outdated Show resolved Hide resolved

geeksilva97 force-pushed the enable-common-sqlite-flags branch from 0c1ec86 to 31c5aa5 Compare March 25, 2025 19:28

TheOneTheOnlyJJ reviewed Mar 26, 2025

View reviewed changes

test/parallel/test-sqlite.js Outdated Show resolved Hide resolved

sqlite: enable common flags

3f64dfa

geeksilva97 force-pushed the enable-common-sqlite-flags branch from 31c5aa5 to 3f64dfa Compare March 26, 2025 13:49

geeksilva97 commented Mar 26, 2025

View reviewed changes

test/parallel/test-sqlite.js Show resolved Hide resolved

cjihrig reviewed Mar 26, 2025

View reviewed changes

test/parallel/test-sqlite.js Outdated Show resolved Hide resolved

sqlite: apply review change

c22f75e

Co-authored-by: Colin Ihrig <[email protected]>

TheOneTheOnlyJJ reviewed Mar 26, 2025

View reviewed changes

test/parallel/test-sqlite.js Outdated Show resolved Hide resolved

TheOneTheOnlyJJ mentioned this pull request Mar 26, 2025

Add dbhash and sqldiff utilities as JS APIs for working with SQLite #57637

Closed

cjihrig approved these changes Apr 3, 2025

View reviewed changes

cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2025

cjihrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed needs-ci PRs that need a full CI run. labels Apr 3, 2025

geeksilva97 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2025

nodejs-github-bot merged commit 9aa57bf into nodejs:main Apr 4, 2025
67 checks passed

cjihrig mentioned this pull request Apr 4, 2025

Enable all SQLite extensions that do not require code changes. #56476

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sqlite: enable common flags #57621

sqlite: enable common flags #57621

geeksilva97 commented Mar 25, 2025 •

edited

Loading

nodejs-github-bot commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig left a comment

geeksilva97 commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025 •

edited

Loading

geeksilva97 commented Mar 25, 2025 •

edited

Loading

codecov bot commented Mar 25, 2025 •

edited

Loading

TheOneTheOnlyJJ left a comment

cjihrig left a comment

TheOneTheOnlyJJ Mar 26, 2025

cjihrig left a comment

nodejs-github-bot commented Apr 3, 2025

nodejs-github-bot commented Apr 3, 2025 •

edited by cjihrig

Loading

TheOneTheOnlyJJ commented Apr 3, 2025

bakkot commented Apr 3, 2025

TheOneTheOnlyJJ commented Apr 3, 2025

cjihrig commented Apr 4, 2025

geeksilva97 commented Apr 4, 2025

nodejs-github-bot commented Apr 4, 2025

sqlite: enable common flags #57621

sqlite: enable common flags #57621

Conversation

geeksilva97 commented Mar 25, 2025 • edited Loading

nodejs-github-bot commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig left a comment

Choose a reason for hiding this comment

geeksilva97 commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025

cjihrig commented Mar 25, 2025

geeksilva97 commented Mar 25, 2025 • edited Loading

geeksilva97 commented Mar 25, 2025 • edited Loading

codecov bot commented Mar 25, 2025 • edited Loading

Codecov Report

TheOneTheOnlyJJ left a comment

Choose a reason for hiding this comment

cjihrig left a comment

Choose a reason for hiding this comment

TheOneTheOnlyJJ Mar 26, 2025

Choose a reason for hiding this comment

cjihrig left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Apr 3, 2025

nodejs-github-bot commented Apr 3, 2025 • edited by cjihrig Loading

TheOneTheOnlyJJ commented Apr 3, 2025

bakkot commented Apr 3, 2025

TheOneTheOnlyJJ commented Apr 3, 2025

cjihrig commented Apr 4, 2025

geeksilva97 commented Apr 4, 2025

nodejs-github-bot commented Apr 4, 2025

geeksilva97 commented Mar 25, 2025 •

edited

Loading

geeksilva97 commented Mar 25, 2025 •

edited

Loading

geeksilva97 commented Mar 25, 2025 •

edited

Loading

codecov bot commented Mar 25, 2025 •

edited

Loading

nodejs-github-bot commented Apr 3, 2025 •

edited by cjihrig

Loading