Add @effect/sql-pglite package#4692
Conversation
🦋 Changeset detectedLatest commit: cb5dfd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
02d328c to
1ea11b9
Compare
|
Not sure why the lint and docgen failed, they pass on my branch locally. |
4cba33b to
5c9e640
Compare
f265f41 to
0fd314b
Compare
|
Thanks for the feedback! I've had to put down my side project using pglite for now but as soon as I can find time I'll update the PR. As for the naming it's extra confusing because PGlite uses |
|
I finally got around to cleaning this up. Sorry for the long delay! |
| const pgDumpMigrations = runPgDump(["--column-inserts", "--data-only", `--table=${table}`]) | ||
|
|
||
| const pgDumpAll = Effect.map( | ||
| Effect.all([pgDumpSchema, pgDumpMigrations], { concurrency: 2 }), |
There was a problem hiding this comment.
Will setting concurrency: 2 here have any effect? Isn't dumping the database a blocking operation?
There was a problem hiding this comment.
The PgliteMigrator is a direct copy of the PgMigrator just with commands switched out for Pglite instead of Pg. I'm not sure why the original PgMigrator does it this way TBH.
|
Any more changes needed or can this be merged now? |
|
I think you probably have to ask Tim for a review manually. Because on my PR, where I had a lot of changes to SQL packages, he was automatically attached as per the CODEOWNERS file, while it seems this automation doesn't work when you are adding a completely new SQL-related package. Maybe you have to even add a new row there |
|
Fixed failing CI in evelant/effect#3 |
|
@tim-smart Whenever you get a chance I'd appreciate your feedback on this so I can make any necessary changes, thanks! |
| executeValues(sql: string, params: ReadonlyArray<Primitive>) { | ||
| // PGlite doesn't have a values() method like postgres.js | ||
| // We'll just return the regular query results | ||
| return this.execute(sql, params, (r) => Object.values(r)) |
There was a problem hiding this comment.
I think you will need an .map here? The results will be an array?
There was a problem hiding this comment.
Maybe I was confused about what executeValues should return. Is it supposed to be an array of arrays of values? That's what this should do since the function param is the row transform, it transforms each row object into an array of values.
There was a problem hiding this comment.
r will be an array of objects here, so you will need to map over each item.
| ) { | ||
| // PGlite doesn't have a cursor method like postgres.js | ||
| // We'll fetch all results at once and convert to a stream | ||
| return Stream.fromEffect( |
There was a problem hiding this comment.
You can use Stream.fromIterableEffect
| constructor(private readonly pg: PGlite) {} | ||
|
|
||
| private run(query: Promise<any>) { | ||
| return Effect.async<ReadonlyArray<any>, SqlError>((resume) => { |
There was a problem hiding this comment.
You can switch to tryPromise here
| extensions: options.extensions ? (client as any) : ({} as any), | ||
| listen: (channel: string) => | ||
| Stream.asyncPush<string, SqlError>((emit) => | ||
| Effect.tryPromise({ |
There was a problem hiding this comment.
You would use Effect.acquireRelease here. I think this would immediately unsubscribe.
| Effect.tryPromise({ | ||
| try: () => client.query(`NOTIFY ${channel}, '${payload}'`), | ||
| catch: (cause) => new SqlError({ cause, message: "Failed to notify" }) | ||
| }).pipe(Effect.map(() => void 0)) |
There was a problem hiding this comment.
| }).pipe(Effect.map(() => void 0)) | |
| }).pipe(Effect.asVoid) |
|
|
||
| test("should work", () => expect(true)) | ||
| describe("PgliteClient", () => { |
There was a problem hiding this comment.
| test("should work", () => expect(true)) | |
| describe("PgliteClient", () => { | |
| describe("PgliteClient", () => { |
|
This is especially needed now that @effect/sql-pg uses |
|
Sorry I dropped the ball on finishing this PR. I'll pick it up again ASAP. |
|
Anything we could help with? 🙂 |
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Tim Smart <hello@timsmart.co>
|
Sorry for taking forever to get back to this. I've been extremely busy. I cleaned everything up, addressed the review comments, rebased on main, and updated everything to match latest changes in sql. @tim-smart if you would take another look I'd appreciate it! |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
hmm, I'm not sure how that happened! Thanks for pointing it out, I'll try to clean it up, although I'm not sure of the best way to do so. Maybe I should copy the pglite changes to a new branch and open a new PR that's clean.
There was a problem hiding this comment.
Hmm yeah IDK what happened here. My history looks clean locally. The commits relating to pglite are clean. It seems that the most recent 5 commits from main are somehow included in the PR after I rebased. I'm not sure why.
There was a problem hiding this comment.
This is because your PR is opened against next-minor branch, and should have been rebased against it. git rebase -i interactive mode is very helpful in such situations, and you can easily remove a few commits.
There was a problem hiding this comment.
Ah. I rebased from main but there are commits there that aren't in next-minor yet so they got picked up. I'm not sure what the right thing is here. Should I make a new PR with the pglite stuff on top of next-minor instead of main?
There was a problem hiding this comment.
No need to make a new branch. Just make git rebase -i HEAD~12 and inside the interactive editor, delete rows with commits you don't want
|
Could this use Also, it seems that |
|
I've somewhat put this on hold as I'm not sure if adding a pglite lib to core effect really makes sense. Pglite is a very useful idea but the progress of the project has been glacial and I'm not sure if it is stable and supported enough to warrant a package in effect. |
|
pglite is essential to the test suite of a codebase I maintain, because it would drastically increase the test run time to spin up a "real" Postgres contanier. I dropped your code from this PR directly into my repo, which made it possible to upgrade effect past the point where Unfortunately this is far enough outside of my usual responsibilities that I can't allocate any time to helping with this PR right now, but for anyone else in the same situation of needing to test their Postgres-dependent effect code, I would suggest pointing your coding agent at this PR and asking it to sort it for you. |
|
Heh of course I made that comment yesterday only to find that electric-sql has just finished their big rework of PGlite. It may be a lot more stable/usable now. I'll try to bring this PR up to speed again soon. |
|
It looks like someone took this and patched it up for v4 and it's now merged, thanks @blntrsz! Going to close this since v4 now has @effect/sql-pglite. |

Type
Description
Add @effect/sql-pglite package. An @effect/sql driver for PGlite, single user postgresql in the browser or node.
Related