fix: emit variant-specific command tag for DEALLOCATE ALL and DISCARD#4315
Conversation
PGAdapter was returning the CommandComplete tag 'DEALLOCATE' for both 'DEALLOCATE name' and 'DEALLOCATE ALL', and 'DISCARD' for every DISCARD variant. Real PostgreSQL emits the subcommand in the tag: 'DEALLOCATE ALL', 'DISCARD ALL', 'DISCARD PLANS', 'DISCARD SEQUENCES', 'DISCARD TEMP'. This matters for pgbouncer (1.22+) running in transaction pooling with prepared-statement support. pgbouncer clears its per-server-connection prepared-statement tracking by sniffing the CommandComplete tag in server.c's handler, with an exact byte-length check and strcmp against 'DEALLOCATE ALL' and 'DISCARD ALL'. Because PGAdapter emitted plain 'DEALLOCATE', the length check failed, pgbouncer did not clear its tracking, and it would subsequently send Bind for a prepared statement name that PGAdapter had just wiped via closeAllStatements(). The client would see 'prepared statement PGBOUNCER_N does not exist' (SQLSTATE 26000) on what looked like a previously-Parsed statement. Return the variant-specific tag to match Postgres's cmdtaglist.h so pgbouncer's tracking stays in sync with PGAdapter's statementsMap.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@praboud-ant This looks generally good to me. Would you mind signing the CLA? Without it, it will not be possible to merge this pull request. |
|
@olavloite yeah, sorry - I'm just waiting on our legal team to add me to the existing corporate CLA. I'll let you know when that goes through. |
|
@olavloite I should be in the corporate CLA now - not sure if you need to boop the build step for it to see that. |
2 participants
Problem
When processing
DEALLOCATE ALL, PGAdapter replies withDEALLOCATE- unlike a real postgres, which replies withDEALLOCATE ALL. A similar problem exists forDISCARD.This matters for clients that validate the response against the expected string; for some clients (in particular: pgbouncer), it causes the client to think that PGAdapter has not actually processed the
DEALLOCATE, which causes incorrect client behavior later on.Fix
Emit the full tag on each response, e.g:
DEALLOCATE ALL, rather thanDEALLOCATE.Tests
New unit tests to validate this behavior.