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

Parameterized queries #17

Closed
samwillis opened this issue Feb 27, 2024 · 14 comments · Fixed by #39
Closed

Parameterized queries #17

samwillis opened this issue Feb 27, 2024 · 14 comments · Fixed by #39

Comments

@samwillis
Copy link
Collaborator

In order to support parameterized queries we need to use the extended postgres wire protocol to pass queries.

@jakeboone02
Copy link

Just wanted to say this project is very cool and I'm looking forward to parameterized queries. I've already added some tests to my react-querybuilder library using PGlite.

@brianc
Copy link

brianc commented Mar 5, 2024

Howdy! Author & long-time maintainer of a node.js postgres client. I semi-recently teased out the wire protocol implementation to a package with zero dependencies outside of node.js itself. I realize requiring node might be a no-go, but if there are changes we could make to work with this lib I'd be happy to help out. At the very least hope it serves as a good point of reference:

https://github.com/brianc/node-postgres/tree/master/packages/pg-protocol

LMK how I can help!

@AntonOfTheWoods
Copy link

@brianc , from the OP on discord (https://discord.com/channels/933657521581858818/1212676471588520006/1212765102332182538) from last week

Hey, I have a clear idea of how this is going to work, and I intend to be working on it next week.
Essentially there is a parallel code path in Postgres to the one we are using that implements the parts of the pg-protocol required.
In a way I'm less concerned about the pg side of things, and more with having an implementation of the pg protocol in js that works in the browser.
My current plan is to use the protocol implementation from node-postgres and port it to the browser (it's currently Node only as it uses the node Buffer api)
I may yet find unexpected challenges. But I'm feeling positive about the timescale.

So I guess if you have some advice on making node Buffer API use optional/pluggable then we'd be off to the races :-).

@brianc
Copy link

brianc commented Mar 6, 2024

I think the buffer usage (node dependency) could be worked around with either a polyfill (babel used to ship a browser compatible polyfill, not sure if they still do?), or I could look into porting things from using node buffer into more low level standardized primitives (typed arrays, etc) at this point. The porting work would take more time & some benchmarking on my side, but using a polyfill should be relatively straight-forward I think. If I need to change things to const Buffer = require('buffer') or something since right now relying on node's global buffer I'm down to do that. You might just try building pg-protocol with webpack + babel & configuring it to polyfill buffer for browser environments. I've had success with this in rosbag.js which did a lot of use of node's buffer construct and worked in the browser, albeit many years ago at this point.

@samwillis
Copy link
Collaborator Author

Hey @brianc 👋 awesome to see you here! I've been meaning to reach out.

You've pretty much summarised my thoughts, I was roughly going to follow these steps:

  1. Use a browser Buffer polyfill to get it working with no changes to pg-protocol.
  2. Investigate changing pg-protocol to explicitly import Buffer so that it can be patched at build time (tiny PR for you). Therefore no need for patching global.
  3. Look at the performance and see if the Buffer polyfill is a bottleneck, if it is then investigate swapping to typed arrays or similar. I'm not really expecting this though, we are likely to have larger perf things elsewhere to optimise first.

Initially this is all for parameterized queries, but I also want to swap the output to use the wire protocol #31. Currently it is using JSON to cross the WASM/JS boundary and we loose type information, Dates for example don't become JS Date objects. So that the next step, but it's much more involved, more changes needed to the C code.

node-postgres is a really great project, and as we add more functionality for PGlite it's certainly going to influence our API design too.

@samwillis
Copy link
Collaborator Author

Not very exciting looking, but I have the beginnings of parameterized queries working:

image

With any luck, and no show stoppers, I hope to get this out next week.

@thruflo
Copy link

thruflo commented Mar 7, 2024

Some inscrutable buffers there!

@brianc
Copy link

brianc commented Mar 8, 2024 via email

@AntonOfTheWoods
Copy link

Definitely let us know if you need some guinea pigs! I haven't been this excited about in browser dB stuff since I first stumbled across electric itself! :)

@samwillis
Copy link
Collaborator Author

@brianc I've only been working on it in Node so far, and pg-protocol has been perfect. I expect we will find that changing pg-protocol to explicitly import Buffer is the route to make this work well in browsers as we can patch in the polyfill when building. Will let you know how I get on when I start testing in browsers.

One key thing I'm aiming for is that the core PGlite lib is self contained with no dependancies, Essentially we will compile them, including pg-protocol, in when building, that way in a JS only "buildless" browser environment it's a single include.

@AntonOfTheWoods very happy to have guinea pigs! We should have something to test early next week, there will be bugs!

@samwillis
Copy link
Collaborator Author

@brianc doesn't look like we need to modify pg-protocol, I have it working by using esbuilds inject 🎉

@brianc
Copy link

brianc commented Mar 9, 2024

ohhh that's awesome!

@samwillis
Copy link
Collaborator Author

Fo anyone following along here, there is much progress in #39 (it uses pg protocol for all IO). There is a dev build attached to the PR.

@AntonOfTheWoods
Copy link

Nice! Is there a feature checklist with progress? Or are we at the "hopefully ok" stage, and now we need to find the things that we didn't know we needed to know stage?

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 a pull request may close this issue.

5 participants