Bare [SET/CREATE TEMP, SELECT] batch on a real PgBouncer still loses session state — and can hang on non-builtin column types #175

Summary

Follow-up to #173 / PR #174. PR #174 fixed the regression #172 introduced for the direct PostgreSQL + pooler_mode=transaction config (one TCP connection = one backend, held) and the temp-table case: a bare batch ["SET ...", "SELECT ..."] (no session_id, no BEGIN) now routes the SELECT to the shared batch client when an earlier statement set session-local state, restoring main's within-batch guarantee.

This issue captures what #174 explicitly deferred: the same bare batch on a real PgBouncer (pool_mode = transaction) still loses session state, and #174's routing additionally reintroduces a #160/#163-style hang on non-builtin result column types under backend reassignment.

The two problems on a real PgBouncer

1. Session state is still lost (pre-existing on main)

On a real PgBouncer transaction pooler, the backend is reassigned per autocommit transaction. A bare ["SET extra_float_digits = 3", "SELECT 1.0/3.0"] runs the SET on backend B1 (autocommit) and the SELECT on B2 (autocommit) — B2 never saw the SET. This is true on main too, so it's not a new regression, but it is the gap the host's editor model ("run whole scripts on one pooled connection so SET @var/temp tables share state") can't satisfy on a real pooler without an explicit BEGIN/COMMIT.

2. #174's shared-client routing can hang on non-builtin types (NEW surface introduced by #174)

#174 routes the SELECT to the shared client with pre_resolve_types = true when session_state_set — i.e. it runs pg_client.prepare(&final_query) then pg_client.query_typed(&final_query) as two separate wire round-trips, with no BEGIN/COMMIT wrapping them (no transaction pins the backend).

This re-runs the prepare-on-shared-cached-client pattern that the removed pre-warm (ca33b98) used, and that #164's one-shot pinned path was built to replace (the s0→s1 named-statement collision across reassignment). The hang mechanism (verified against the tokio-postgres 0.7.18 source):

• exec_query_on_client with pre_resolve_types=true calls prepare(&final_query). For a result column with a non-builtin type (custom enum/composite/extension type), prepare calls get_type (prepare.rs:135), which on a cache miss fires the internal named typeinfo_statement across 2+ Sync boundaries (prepare.rs:144).
• On a real PgBouncer with no open transaction, each Sync boundary is an autocommit boundary — the pooler can reassign the backend during the prepare's typeinfo_statement. If it does, the prepare returns 42P05, swallowed by .ok() (query.rs pre-resolve block) — the type cache stays cold.
• The subsequent query_typed's get_type (query.rs:102, in the RowDescription loop) is then a miss → fires typeinfo_statement mid-stream (the DataRows are already queued behind RowDescription) → query_typed won't drain them until get_type returns → deadlock once the row data fills the bounded responses channel + socket buffer (the #160/#163 mechanism).
• For small results it surfaces as a 42P05; for large results it hangs until the host's 120s RPC timeout.

Important nuance (why it's conditional, not unconditional): a successful prepare warms CachedTypeInfo.types (prepare.rs:183 client.set_type), so query_typed's get_type hits client.type_(oid) and returns early — no typeinfo_statement mid-drain, no hang. The hang only triggers when the prepare's own typeinfo_statement reassigns (cache stays cold). So it requires: real PgBouncer + backend reassignment during the prepare + non-builtin column types + a large enough result to fill the buffer.

On #172 (the base #174 branches from), the same SELECT took the one-shot pinned path — the SET was lost (wrong precision) but no hang. So #174 trades a wrong-data bug for a possible hang on real PgBouncer for non-builtin types.

Reproduction (requires a real PgBouncer, not in this repo's CI)

A real PgBouncer transaction pooler (pool_mode = transaction) with traffic causing backend reassignment:

queries: ["SET extra_float_digits = 3", "SELECT id, status FROM orders WHERE id < 10000"]
//            where `status` is a custom enum type and the result is large (>~600 rows)
pooler_mode: "transaction"

Not exercised by #174's two live regression tests — both use only builtin column types (int, text), where get_type is a Type::from_oid cache hit and typeinfo_statement never fires. The existing wide_enum_table_returns_rows_via_non_session_batch_on_pooler test covers the enum case but with a single query (no preceding SET), so session_state_set stays false and it routes to the safe one-shot path.

Scope / why #174 didn't fix this

#174 scopes its fix to the direct PostgreSQL + pooler_mode=transaction config (one held TCP connection = one backend, no reassignment → the unpinned prepare is safe) and the temp-table case. The real-PgBouncer GUC loss is pre-existing on main and was explicitly out of scope. The PR's routing comment now documents the real-PgBouncer tradeoff honestly; a code fix was deferred here.

Suggested fix directions (need their own design)

A proper fix is a real-PgBouncer-support design decision, not a bolt-on. Options:

• Transaction pin for the session-state SELECT: BEGIN on the shared client before the SELECT, COMMIT after — pins the backend so the prepare is safe and the SET/temp table is visible on the same backend. Fixes both Stage repo scaffolding ahead of postgres-plugin migration #1 and Align README scaffolding with sibling plugin repos #2. Cost: changes autocommit/atomicity semantics (the whole batch becomes transactional — a mid-batch error cascades 25P02), a parity deviation from main, and entangles with the open #170 (25P02 transaction-abort cascade on the pooler in_transaction path). This is "option B" from the Bare [SET/CREATE TEMP, SELECT] batch on a transaction pooler loses session-local state on the one-shot path #173 design discussion — the tradeoffs that made us choose fix: keep bare [SET/CREATE TEMP, SELECT] batch on the shared client on a transaction pooler (#173) #174's narrower fix apply.
• One-shot fallback when the result has non-builtin types: detect non-builtin column types (or always) and fall back to the #164 one-shot pinned path for the SELECT, accepting that the SET/temp table is lost on a real pooler. Fixes Align README scaffolding with sibling plugin repos #2 (no hang) but not Stage repo scaffolding ahead of postgres-plugin migration #1 (still loses state — same as main). Cheaper than the transaction pin.
• Surface a clear "wrap in BEGIN/COMMIT" nudge for bare [SET, SELECT] batches on a transaction pooler, rather than silently-wrong results or a hang. Cheapest; loses the within-batch guarantee but makes the failure legible.

\cc @aesslinger