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

New method execProtocolRaw() #127

Merged
merged 3 commits into from
Jul 27, 2024

Conversation

gregnr
Copy link
Contributor

@gregnr gregnr commented Jul 26, 2024

When passing raw message data directly to PGlite via execProtocol() (through pg-gateway), pg protocol error messages end up getting thrown by PGlite which means:

  • I have to re-create the protocol error message from the DatabaseError object
  • We lose any other messages that PG sent back (often a ReadyForQuery message)

Ideally there would be a way to execProtocol without the wrapping logic for this kind of use case. This PR proposes an execProtocolRaw() method that simply forwards raw message data in and returns the raw message result from PG.

Alternatively, if there is a way for execProtocol to not throw the DatabaseError and/or return the full raw message in the event of an error, that should work too.

packages/pglite/src/pglite.ts Outdated Show resolved Hide resolved
packages/pglite/src/pglite.ts Outdated Show resolved Hide resolved
@gregnr
Copy link
Contributor Author

gregnr commented Jul 26, 2024

@samwillis Updated location of syncToFs() and removed the mutex

@gregnr
Copy link
Contributor Author

gregnr commented Jul 26, 2024

One more quick thought - is there any reason why we don't await #checkReady() at the beginning of execProtocol (and now also execProtocolRaw)? Should we add this?

@samwillis
Copy link
Collaborator

One more quick thought - is there any reason why we don't await #checkReady() at the beginning of execProtocol (and now also execProtocolRaw)? Should we add this?

execProtocol is used by the private #runQuery and #runExec methods, the intention is that these can be used inside #init during startup (#runExec is used). If we use #checkReady inside execProtocol we would be unable to use it or those private methods inside init as the database isn't marked as ready until its completion.

My thinking was that anyone dropping down to execProtocol would be ok with having to explicitly await the waitReady promise.

@gregnr gregnr requested a review from samwillis July 27, 2024 03:48
@samwillis samwillis merged commit 86cb410 into electric-sql:main Jul 27, 2024
1 check failed
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.

2 participants