fix: avoid pinning failed advisory try locks

[ewhauser]

Summary

• inspect pg_try_advisory_lock* result rows before merging optimistic lock bookkeeping
• avoid pinning a transaction-pooled client when a directly returned try-lock result is false
• preserve pinning for acquired, shared, reentrant, and transaction-spanning session locks
• add focused Rust unit and integration regression coverage for simple and extended protocol paths

PgDog previously merged advisory-lock intent from the parsed statement at ReadyForQuery, regardless of whether a try-lock succeeded. The parser now records the result columns for directly returned session try-lock calls, and the query engine inspects only those DataRow fields. It accounts for text and binary results, multiple result columns, and repeated rows without adding another backend query. Missing or undecodable results remain conservatively pinned.

Fixes #1578.

Validation

• reproduced the failure with the real v0.1.56 PgDog binary and cached extended protocol
• cargo test -p pgdog frontend::client::query_engine::test::advisory_lock -- --test-threads=1 (15 passed)
• focused parser advisory-lock tests (22 passed)
• Rust integration test target compiled with cargo test -p integration_tests_rust --test mod integration::advisory_lock::test_failed_try_advisory_lock_does_not_pin_client --no-run
• cargo clippy -p integration_tests_rust --test mod --no-deps -- -D warnings
• cargo check -p pgdog
• cargo build -p pgdog
• git diff --check

Notes

• Result inspection is enabled only for parsed session try-lock calls whose boolean is returned directly (optionally through casts). More complex expressions remain conservatively optimistic.
• The live PgDog binary validation was on Darwin ARM64 against PostgreSQL 16.15 on Linux ARM64. The PgDog binary itself has not yet been exercised on Linux ARM64.

[levkk]

The advisory lock query will return an error if the lock failed to get acquired. It's much easier for us (and we do this already) to inspect the error we receive and remove the lock from our optimistic data structure.

[ewhauser]

Thanks. I may be misunderstanding the suggested path, but PostgreSQL does not return an error when pg_try_advisory_lock* cannot acquire a contended lock - it returns a normal boolean false result.

Inspecting errors would cover actual execution failures, but not lock contention. Would you prefer that PgDog inspect the returned false value instead of reconciling through pg_locks? I used reconciliation because it remains correct for binary results, multiple result columns, nested lock calls, and reentrant locks, but I’m happy to align with the preferred result-inspection design.

[levkk]

Nope, it's me who completely misunderstood 🤣 Sorry, getting late in the week (albeit it's only Thursday..). Yes, I think you're right, we need to inspect the result (we support parsing DataRow messages already at least), and make sure the boolean is set.

Running an extra query is too expensive, inspecting the result is cheap. Do make sure to do that if and only if we are running an advisory lock query (maybe flag it in the context) so we don't have to inspect all results we receive.

[ewhauser]

Thanks — updated in ec10acb.

PgDog now records the directly returned pg_try_advisory_lock* result columns in the parsed route and inspects only those DataRow fields. A false result is excluded from the optimistic lock set; true remains pinned. The result decoder follows the requested text/binary format, including when Bind and Execute are separated by Flush, and maps multiple result columns and repeated rows. Missing or undecodable results remain conservative.

The pg_locks reconciliation query has been removed, so this adds no backend round trip and ordinary result rows are not inspected.

[levkk]

I'm assuming I'm talking to your agent here, so I'm gonna be pretty demanding :) Can you move this to AdvisoryLocks structure just above this?

[ewhauser]

Half and half but be as demanding as you want :)

[ewhauser]

Done in ec10acb — all transient try-lock result state now lives in AdvisoryLocks.

[levkk]

Could you move this to its own method?

[ewhauser]

Done in ec10acb — DataRow inspection is now encapsulated in AdvisoryLocks::process_data_row.