Skip to content

refactor: use pragma method in better-sqlite3 #10684

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.

Sign up for GitHub

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

Jump to bottom
Merged
gioboa merged 2 commits into from
Dec 1, 2025
Merged

refactor: use pragma method in better-sqlite3 #10684

gioboa merged 2 commits into from
Dec 1, 2025
+12 −11

Conversation

@mohd-akram
Copy link
Contributor

@mohd-akram mohd-akram commented Feb 6, 2024

Description of change

Use the pragma method in better-sqlite3. This is the recommended way to run pragmas.

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@mohd-akram
refactor: use pragma method in better-sqlite3
586bc3b 
This is the recommended way to run pragmas.
gioboa
gioboa reviewed Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @mohd-akram for your help.
Are any drawbacks about this change?

@mohd-akram
Copy link
Contributor Author

mohd-akram commented Jan 22, 2025

@gioboa No drawbacks that I'm aware of.

@gioboa
Copy link
Collaborator

gioboa commented Jan 22, 2025

@gioboa No drawbacks that I'm aware of.

Thanks 👍

@alumni
Copy link
Collaborator

alumni commented Jan 22, 2025 •
edited
Loading

Are any drawbacks about this change?

If you want to use bun's sqlite driver (similar to better-sqlite3), it won't have the pragma method 😛

Bun-related issue: #11236

gioboa
gioboa reviewed Jan 24, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is not working with bun 🤔
An idea is to check the runtime and create a condition in the code eg. detect Bun.
What do you think?

@alumni
Copy link
Collaborator

alumni commented Jan 24, 2025 •
edited
Loading

We don't support bun at the moment, so my comment was there just in case we would like to 🤷🏻

I played a bit with bun:sqlite and I was able get it to work with typeorm for a basic use case with only with 2 minor changes (see the linked issue).

Later edit: bun has a ticket to add support for .pragma: oven-sh/bun#2023

@angelxmoreno
Copy link

angelxmoreno commented Apr 18, 2025

what is the status of this PR? have the admins agreed or disagreed to review/merge this work?

gioboa
gioboa approved these changes Apr 18, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me
Thanks @mohd-akram

@seepine seepine mentioned this pull request Sep 20, 2025
Open
18 tasks
@qodo-free-for-open-source-projects
Copy link

qodo-free-for-open-source-projects bot commented Dec 1, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 1, 2025

commit: d435d68

@coveralls
Copy link

coveralls commented Dec 1, 2025

Coverage Status

coverage: 80.758%. remained the same
when pulling d435d68 on mohd-akram:improve-better-sqlite3
into c4f5d12 on typeorm:master.

gioboa
gioboa reviewed Dec 1, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are fine.
What's the benefit of using pragma ? is it faster? reliable?

@gioboa gioboa requested review from G0maa, alumni and naorpeled December 1, 2025 20:43
@alumni
Copy link
Collaborator

alumni commented Dec 1, 2025

hmm, better-sqlite3 does not mention exactly what:

It's better to use this method instead of normal prepared statements when executing PRAGMA, because this method normalizes some odd behavior that may otherwise be experienced.

https://github.com/WiseLibs/better-sqlite3/blob/master/docs/api.md#pragmastring-options---results

gioboa
gioboa reviewed Dec 1, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your help @mohd-akram

@gioboa gioboa merged commit ec3ea10 into typeorm:master Dec 1, 2025
64 checks passed
@mohd-akram mohd-akram deleted the improve-better-sqlite3 branch December 1, 2025 21:44
ThbltLmr pushed a commit to ThbltLmr/typeorm that referenced this pull request Dec 2, 2025
@mohd-akram @ThbltLmr
refactor: use pragma method in better-sqlite3 (typeorm#10684)
d431890
@Minishlink Minishlink mentioned this pull request Jan 12, 2026
Open
mgohin pushed a commit to mgohin/typeorm that referenced this pull request Jan 15, 2026
@mohd-akram
refactor: use pragma method in better-sqlite3 (typeorm#10684)
2352fd3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gioboa gioboa gioboa approved these changes

@alumni alumni alumni approved these changes

@naorpeled naorpeled Awaiting requested review from naorpeled

@G0maa G0maa Awaiting requested review from G0maa

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mohd-akram @gioboa @alumni @angelxmoreno @coveralls