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

Proof of concept: River operating with pgx simple protocol #234

Closed
wants to merge 1 commit into from

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Feb 25, 2024

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 for
jsonbs 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 prevents
pgx 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 carries
significant performance penalty because it's basically the same
underlying structure as far as Go is concerned.

[1] #205 (comment)

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 for
`jsonb`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 prevents
pgx 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* carries
significant performance penalty because it's basically the same
underlying structure as far as Go is concerned.

[1] #205 (comment)
@brandur
Copy link
Contributor Author

brandur commented Mar 17, 2024

Closing this out for now.

@brandur brandur closed this Mar 17, 2024
@brandur brandur deleted the brandur-simple-protocol branch March 17, 2024 23:00
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.

1 participant