Proof of concept: River operating with pgx simple protocol #234
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had an issue opened a little while back (#171) wherein it turned out
that River couldn't operate with pgx's simple protocol turned on, which
appeared to be because the simple protocol doesn't know anything about a
target Postgres tuple's makeup and therefore had to guess how to encode
parameters based on input types, which was leading
[]byte
intended forjsonb
s to be binary encoded, which isn't allowed.We closed that issue after I pointed out that PgBouncer now supports
prepared statement pooling, and therefore simple protocol is no longer
necessary.
We recently got another comment [1] where someone had tried to use River
with Supavisor on Supabase, which also does not supported prepared
statements, leading to lots of errors. Normally, pgx's simple protocol
could be used to address this, but as we saw in #171, River doesn't
support that.
Here, a proof of concept demonstrating how River could support simple
protocol by modeling
jsonb
fields in sqlc as strings, which preventspgx from encoding to binary with simple procotol, and we get to a point
where all driver tests are passing.
I'm not sure if we should action on this one or not. Having to fall back
to
string
fields a bit gross, although I don't think carriessignificant performance penalty because it's basically the same
underlying structure as far as Go is concerned.
[1] #205 (comment)