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 generic sql driver (via db0) #476

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

zsilbi
Copy link

@zsilbi zsilbi commented Sep 4, 2024

Hello!

Resolves #400

I added tests for libsql (local file) and postgresql (only runs when the connection string in the .env is present.
And I also updated the docs.

Notes:

Inlining db0 in the vitest config was necessary because of this error from the postgres test
FAIL  test/drivers/db0.test.ts [ test/drivers/db0.test.ts ]
SyntaxError: Named export 'Client' not found. The requested module 'pg' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'pg';
const { Client } = pkg;

Related: unjs/db0#62 unjs/db0#63

@pi0
Copy link
Member

pi0 commented Sep 4, 2024

Thanks for the PR dear @zsilbi it is a good start.

TBH i have to go over it for next steps (and we need a newer db0 release since it is stall in current experimental version) but some pointers if you can help in the meantime:

  • Table creation should be automated too so that this driver can be used in zero config setups (we might need to have migration too later as a note)
  • I would remove dotenv requirement. pglite and sqlite can be used for testing

@pi0 pi0 marked this pull request as draft September 4, 2024 15:30
@pi0 pi0 changed the title feat: add db0 driver feat: add generic sql driver (via db0) Sep 4, 2024
src/drivers/db0.ts Outdated Show resolved Hide resolved
@zsilbi
Copy link
Author

zsilbi commented Sep 4, 2024

Table creation should be automated too so that this driver can be used in zero config setups

Should it always try to create the table before the first query, or only after the first failed query and then retry?

To make it easier to configure, dialect entries could also be added to db0 connectors.
The database instance could return the dialect of the provided connector so this driver could use that.

@pi0
Copy link
Member

pi0 commented Sep 4, 2024

or only after the first failed query and then retry?

i wasn't thinking of it sounds a nice idea!

To make it easier to configure, dialect entries could also be added to db0 connectors.

i was thinking of it actually! feel free to open an issue in db0 to support this.

@zsilbi zsilbi mentioned this pull request Sep 4, 2024
1 task
@zsilbi
Copy link
Author

zsilbi commented Sep 4, 2024

or only after the first failed query and then retry?

i wasn't thinking of it sounds a nice idea!

I got this working. It also stores the promise of the CREATE TABLE query, so concurrent queries could queue up waiting for the same promise to be fulfilled.

I would remove dotenv requirement. pglite and sqlite can be used for testing

I'll change it to use pglite when it will be available: unjs/db0#110

@zsilbi zsilbi marked this pull request as ready for review October 10, 2024 20:24
@zsilbi zsilbi requested a review from pi0 October 10, 2024 20:24
Copy link

codecov bot commented Oct 13, 2024

Codecov Report

Attention: Patch coverage is 86.92308% with 17 lines in your changes missing coverage. Please review.

Project coverage is 59.87%. Comparing base (4d61c78) to head (e5abf65).
Report is 80 commits behind head on main.

Files with missing lines Patch % Lines
src/drivers/db0.ts 86.82% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   65.05%   59.87%   -5.18%     
==========================================
  Files          39       40       +1     
  Lines        4055     3270     -785     
  Branches      487      555      +68     
==========================================
- Hits         2638     1958     -680     
+ Misses       1408     1303     -105     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Generic SQL driver with db0
2 participants