Open
Conversation
bgentry
force-pushed
the
bg/simple-protocol-jsonb-mode
branch
from
456d32a to
e9968c1
Compare
bgentry
force-pushed
the
bg/simple-protocol-jsonb-mode
branch
from
e9968c1 to
199a4dc
Compare
River passes marshaled JSON query inputs as `[]byte` into sqlc-generated `pgx` calls. In `QueryExecModeSimpleProtocol` and `QueryExecModeExec` (including PgBouncer transaction-pooling setups), pgx treats `[]byte` bind args as `bytea`, so `json`/`jsonb` inputs can fail with invalid JSON syntax. This commit adapts JSON bind arguments from `[]byte` to `string` at wrapper `Exec`/`Query`/`QueryRow` boundaries, but only for pgx text execution modes. Query option parsing now mirrors pgx option semantics so per-query `QueryExecMode` overrides are honored. When a `pgx.QueryRewriter` is present, the driver wraps it so adaptation runs after rewrite against final SQL and bind args. Explicit binary placeholders cast as `$n::bytea` or `CAST($n AS bytea)` are excluded from adaptation so intentional `bytea` inputs keep working in both extended and simple protocol paths. `defaultQueryExecMode` stays alongside `templateReplaceWrapper`, and the test-only `SharedTx.Conn()` now returns `nil` for capability probing instead of forcing panic matching. Coverage includes driver tests for per-query mode overrides, `QueryRewriter` post-rewrite adaptation, `Exec`-path behavior, explicit `bytea` cast protection, and nil-conn fallback. `riverdrivertest` now exercises pgx endpoints in default, simple-protocol, and exec-mode configurations. Fixes #1153.
bgentry
force-pushed
the
bg/simple-protocol-jsonb-mode
branch
from
199a4dc to
61c06f6
Compare
Contributor
Author
|
@casparwylie any chance you might be able to try this out on a dev setup to see if it works smoothly for you? |
|
Just ran against the exact environment that was failing, and this does indeed fix the issue! Thank you |
brandur
approved these changes
Feb 26, 2026
Contributor
brandur
left a comment
brandur
left a comment
There was a problem hiding this comment.
Very cool! Another long standing problem bites the dust!
| exerciseDriverRiverPgxV5WithMode(t, pgx.QueryExecModeCacheStatement) | ||
| }) | ||
|
|
||
| // PgBouncer transaction-pooling compatibility path (`simple protocol`). |
Contributor
There was a problem hiding this comment.
Nit, but is "simple protocol" in backticks like this intended? It seems like maybe it should've been just simple protocol without quotes, or something like pgx.QueryExecModeSimpleProtocol. It's repeated twice more below too which is what drew it to my attention.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
River passes marshaled JSON query inputs as
[]byteinto sqlc-generatedpgxcalls. InQueryExecModeSimpleProtocolandQueryExecModeExec(including PgBouncer transaction-pooling setups), pgx treats[]bytebind args asbytea, sojson/jsonbinputs can fail with invalid JSON syntax.This change adapts JSON bind arguments from
[]bytetostringat wrapperExec/Query/QueryRowboundaries, but only for pgx text execution modes. Query option parsing now mirrors pgx option semantics so per-queryQueryExecModeoverrides are honored. When apgx.QueryRewriteris present, the driver wraps it so adaptation runs after rewrite against final SQL and bind args.Explicit binary placeholders cast as
$n::byteaorCAST($n AS bytea)are excluded from adaptation so intentionalbyteainputs keep working in both extended and simple protocol paths.defaultQueryExecModestays alongsidetemplateReplaceWrapper, and the test-onlySharedTx.Conn()now returnsnilfor capability probing instead of forcing panic matching.Coverage includes driver tests for per-query mode overrides,
QueryRewriterpost-rewrite adaptation,Exec-path behavior, explicitbyteacast protection, and nil-conn fallback.riverdrivertestnow exercises pgx endpoints in default, simple-protocol, and exec-mode configurations.Fixes #1153.