Skip to content

Direct-path pre-resolve .ok() swallow can reintroduce the hang; no plugin-level timeout (asymmetry with pooler path) #170

Open

Description

@aesslinger

Summary

The #166 fix (PR #168) pre-resolves result column types via prepare on direct connections, but swallows the prepare error via .ok(). If prepare fails for a non-builtin column type, query_typed runs without the type cache warm → get_type fires typeinfo_statement mid-stream → the same deadlock the PR is fixing. Unlike the pooler path (#164's exec_query_pooled_txn), the direct path has no plugin-level timeout to bound the hang — the user sees the opaque host 120s timeout with no actionable guidance.

The asymmetry

  • Pooler path (run_pinned_body, query.rs:503): surfaces the prepare error (return Err(format_query_error(...))) and wraps the whole sequence in a 90s POOLED_TXN_TIMEOUT. A prepare failure is a clean, actionable error; a hang is bounded to 90s.
  • Direct path (query.rs:728): prepare(...).await.ok() swallows the error, and exec_query_on_client has no timeout. A prepare failure for a non-builtin type silently proceeds to a deadlock with no bound except the host's 120s.

How real is it

Requires prepare to fail for a non-builtin column type on a direct connection — rare (a DBA revoking SELECT on pg_type, or a transient catalog error during the Describe phase). Not a regression: rc.11 hangs always on non-builtin large results (no pre-resolve at all); this PR fixes it when prepare succeeds (the common case) and falls back to rc.11 behavior when prepare fails (the rare case). It's strictly better than rc.11 — it just doesn't fully close the window.

Suggested fix direction

The cleaner fix is a direct-path query timeout: wrap the query_typed (and the pre-resolve prepare) in a tokio::time::timeout like the pooler path's POOLED_TXN_TIMEOUT, so a hang surfaces as an actionable error after ~90s regardless of whether the pre-resolve succeeded. This bounds the worst case without changing the .ok() semantics (which are correct for builtin types and avoid regressing utility commands that prepare rejects but query_typed handles).

This is related to the deferred acquisition-timeout gap (the unbounded pool.get().await on all pool paths) — both are "the direct/pool path lacks the bounds the pooler pinned path has."

Scope

Not introduced by #168 (the swallow is strictly better than rc.11's always-hang). Filed as a follow-up so the direct-connection fix can land without expanding scope into direct-path timeout bounding.


Update 2026-10-23 — second consequence of the .ok() swallow: 25P02 transaction-abort cascade (found by the PR #172 pre-handoff review)

PR #172 (#169 fix) extends the prepare().ok() swallow to a NEW path — the pooler in_transaction batch path (run_batch_in_session passes pre_resolve_types=true when in_transaction). On that path the swallow has a second failure mode beyond the hang #170 already tracks:

If the user's query has a Parse-time error (syntax error, nonexistent table, permission denied) inside an open transaction, the swallowed prepare aborts the transaction (PostgreSQL puts any transaction block into failed state after any command error). The subsequent query_typed then fails with SQLSTATE 25P02 "current transaction is aborted, commands ignored until end of transaction block" instead of the real, actionable error (e.g. 42P01 "relation does not exist"). format_query_error (src/client.rs:114-134) only rewrites 26000/42P05, so 25P02 passes through verbatim.

Scenario

  1. Batch 1: BEGIN on a pooler_mode=transaction connection with session_id set (in_transaction=true).
  2. Batch 2 (same session): SELECT * FROM nonexistent_table
    • main (rc.11): prepare was gated on !is_transaction_pooler (false for poolers), so query_typed ran first and returned the real 42P01 "relation does not exist."
    • PR fix: pre-resolve column types on batch/session pooler path (#169) #172: prepare(...) runs first (pre_resolve_types=true), fails with 42P01, aborts the txn, error swallowed by .ok(). Then query_typed fails with 25P02. The user sees the confusing cascade, and all remaining batch statements in the same transaction also fail with 25P02 until ROLLBACK.
  3. transaction_effect("SELECT")=None keeps in_transaction=true, so the connection is re-pinned in the aborted state — the next batch also sees 25P02 until the user issues ROLLBACK.

Why this belongs here, not a new issue

This is the same .ok() swallow #170 already documents, just a different consequence on a newly-enabled path. The pre-existing confusing-25P02 pattern already exists for direct connections (which already ran prepare().ok() under #168), so PR #172 extends rather than invents the pattern — but it newly reaches the pooler in_transaction path, which is live in production once the host ships pooler_mode (tabularis#1128, merged 2026-10-17).

Fix direction (shared with the original #170 work)

A direct-path query timeout (the #170 suggested fix) bounds the hang consequence but does not fix the 25P02 consequence — the swallowed prepare still aborts the txn before the timeout fires. The cleaner shared fix handles both: surface the prepare error (or at least don't run query_typed after a prepare that aborted the transaction) on paths where the prepare runs inside an open transaction, AND bound the whole sequence with a timeout. Worth designing together so the two consequences of the same swallow are fixed coherently rather than piecemeal.

No data loss — recoverable via ROLLBACK; impact is error-message quality (confusing cascade hides the root cause). But it will be hit by users who typo a query inside a transaction on a pooler.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions