fix: fold search path into set config

[ferhatelmas]

What kind of change does this PR introduce?

refactor for performance

What is the current behavior?

If pool is external, we do an extra round trip for search path.

What is the new behavior?

Fold it into scope query similar to statement timeout (#1154).

Additional context

Also added a fast path for no signal to reduce GC overhead.
Related to #1154

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[ferhatelmas]

@claude review always

[claude]

I didn't find any bugs, but this PR touches core transaction/connection-pooling logic (search_path and statement_timeout folding, plus the abort-signal fast path in runPgQuery) that runs on every tenant request, so I think it's worth a human look before merging.

What was reviewed:

• Verified the $N placeholder ordering and values arrays line up correctly across all four SQL variants (search_path only, timeout only, both, neither) in both getPendingSettingsSql and getScopeConfigSql.
• Confirmed pendingSearchPath/pendingStatementTimeoutMs are always consumed via setScope, since every call site (StoragePgDB.withTransaction, runQuery) invokes setScope immediately after connection.transaction(), so search_path won't silently go unset.
• Checked that the removed rollbackTransactionSafely path (previously used to roll back on a failed eager search_path setup query) has no remaining callers now that search_path setup is folded into scope/first-query instead of an eager query.
• Checked the new no-signal fast path in runPgQuery preserves the same connection-state-error disposal behavior as the abort-signal path.

Extended reasoning...

Overview

This PR extends the deferred-setting-folding pattern introduced in #1154 (statement_timeout) to also fold search_path into the scope/first-query set_config call for external-pool tenant connections, eliminating a previously-eager separate round trip. It also removes the now-dead rollbackTransactionSafely error path that existed only to roll back on a failed eager search_path setup query, and restructures runPgQuery with a fast path for the common case of no abort signal (skips creating the abort Promise/listener).

Security risks

None identified. The search_path/timeout values are passed as parameterized $N arguments, not interpolated into SQL text, and the search_path itself is a static server-side config value (not user input), so there's no injection surface. No auth/permission logic is touched.

Level of scrutiny

This is core database connection/transaction code shared by every tenant request (PgTenantConnection, PgTransaction, runPgQuery), so I treated it as production-critical and read through the full pending-value lifecycle (constructor defaults → lazy consumption via query()/setScope() → SQL builder branch selection) plus the abort-signal restructuring, rather than a quick skim. Despite finding it internally consistent, code at this level of criticality and this much structural change (new state fields, removed error-handling path, control-flow restructuring in the query executor) benefits from a second pair of eyes before merging.

Other factors

Test coverage is thorough and mirrors the existing statement_timeout tests structure closely (new tests for search_path-only, combined, and setup-query-should-not-consume-pending-state cases), and one now-obsolete test (rollback-on-search_path-setup-failure) was removed along with the code path it exercised. No bugs were found by the automated bug hunter or my own reading, but given the blast radius of this file I'm deferring rather than shadow-approving.

[coveralls]

Coverage Report for CI Build 29853927871

Coverage increased (+20.7%) to 79.825%

Details

• Coverage increased (+20.7%) from the base build.
• Patch coverage: 57 of 57 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	12814
Covered Lines:	10675
Line Coverage:	83.31%
Relevant Branches:	7503
Covered Branches:	5543
Branch Coverage:	73.88%
Branches in Coverage %:	Yes
Coverage Strength:	406.33 hits per line

---

💛 - Coveralls

[claude]

No bugs found; deferring for human review given this refactors core transaction/scope-setup logic that governs tenant isolation (search_path, role, RLS session settings) in the shared Postgres connection layer.

What was reviewed: the new getPendingSettingsSql/getScopeConfigSql branching and parameter-index alignment across all statement_timeout/search_path combinations, the withDefaultTransactionSettings option-merging for external vs. internal pools, the take/consume semantics preventing double-application, and the runPgQuery no-signal fast path refactor. Also confirmed the pre-existing gap noted in the ruled-out candidate issue (low-level beginTransaction() not applying external-pool search_path) predates this PR and isn't a regression.

Extended reasoning...

This PR removes a separate search_path round-trip for external pools by folding it into the same set_config statement used for statement_timeout and scope, mirroring the prior statement_timeout-folding work (#1154). The change touches PgTransaction, PgTenantConnection.transaction/setScope, and the shared query-execution helper (runPgQuery), which are used on effectively every tenant database request.

Security risks

search_path and role/session scoping underpin schema/tenant isolation and RLS enforcement, so mistakes here (wrong param ordering, dropped settings, re-application bugs) could have isolation implications. I traced the SQL-branch helpers and parameter indices for all four combinations (none/timeout/search_path/both) and they line up correctly with the values arrays and with the extensive new/updated unit tests. No injection risk — all values are parameterized, never interpolated into SQL text.

Level of scrutiny

This is a non-trivial refactor of shared, production-critical connection/transaction code (not a config tweak), so it warrants a human look despite the absence of concrete bugs. It also deletes an existing safety path (rollback-on-search_path-setup-failure) since that failure mode no longer exists after folding, which is a reasonable but non-obvious behavioral change worth a maintainer sign-off.

Other factors

The PR has strong test coverage (coveralls reports 100% patch coverage) and includes many new unit tests covering the folded-settings combinations, empty/invalid normalization, and non-reapplication after setScope consumes pending state. A bug-hunting pass and my own review found no correctness issues; the one candidate issue raised (beginTransaction never applying external-pool search_path) was confirmed pre-existing rather than introduced by this change.