[ganzyukv]

Why

Session-level SET search_path leaks between clients behind PgBouncer in transaction pooling: the next transaction runs on a server connection that still has another client's schema, so preview requests read and write the wrong schema.

What Changed

• Opt-in SearchPathMode::Transaction: schema applied with SET LOCAL
• DBAL driver middleware covering queries, exec and prepared statements
• Schema commands skip the session SET in transaction mode
• Integration tests on PostgreSQL 14/18 behind PgBouncer, new CI job
• README docs; phpcs ^3.13 (3.7 is blocked by a Composer advisory)

Alternatives & Trade-offs

PgBouncer session pooling was rejected: one server connection per client, max_connections grows with autotests. Skipping SET LOCAL for the default schema saves round trips but inherits leaked session paths, so strict is the default.

Risk & Blast Radius

• Middleware wraps every connection; default Session mode passes calls through unchanged
• Transaction mode adds two round trips per statement outside a transaction
• Non-transactional statements (CREATE INDEX CONCURRENTLY) fail for schemas that need SET LOCAL

Reviewer Guide

1. TransactionSearchPathConnection: implicit transaction, savepoint and raw transaction-control handling.
2. ~1000 lines, mostly integration tests and README; the core logic is ~250 lines.

Context & Links

• PLT-13586
• PLT-13585
• PLATECO-7724

---

_{Description composed by pr-why}

🤖 Generated with Claude Code

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues

• Doctrine\\DBAL\\Driver\\Connection methods beginTransaction(), commit(), and rollBack() are… · New
• Granting pull-requests: write to the entire job broadens permissions for all steps (including… · New
• Executing multiple statements in a single exec() call (BEGIN; SET LOCAL ...) is less portable… · New
• The boolean passed through the middleware is documented as “whether the transaction was opened by… · New
• This integration test suite uses DIRECT_DATABASE_URL in multiple helpers… · New

What changed in this PR

Adds an opt-in transaction-scoped search_path mode to prevent schema leakage when using PgBouncer in transaction pooling by applying schema via SET LOCAL per transaction/statement.

Changes:

• Introduces SearchPathMode::{Session,Transaction} and wiring in SchemaConnection to enable transaction-safe schema application.
• Adds DBAL middleware (TransactionSearchPathDriver/Connection/Statement) to apply SET LOCAL search_path for queries, exec, and prepared statements.
• Adds PgBouncer-backed integration tests, CI service setup, and documentation updates for the new mode and limitations.

File	Description
src/​Doctrine/​SearchPathMode.php	Defines the new search_path mode enum.
src/​Doctrine/​SchemaConnection.php	Integrates transaction-mode selection and middleware driver wrapping.
src/​Doctrine/​Middleware/​TransactionSearchPathDriver.php	Wraps driver connections and rejects unsupported native pgsql driver in transaction mode.
src/​Doctrine/​Middleware/​TransactionSearchPathConnection.php	Applies SET LOCAL search_path per transaction/statement and handles tx-control SQL.
src/​Doctrine/​Middleware/​TransactionSearchPathStatement.php	Ensures prepared statements also run with applied transaction search_path.
src/​Command/​Doctrine/​AbstractDoctrineSchemaCommand.php	Skips session-level SET search_path when transaction mode is enabled.
src/​Exception/​UnsupportedDriverException.php	Adds a dedicated exception for unsupported drivers in transaction mode.
tests/​Doctrine/​SchemaConnectionTest.php	Adds unit tests for transaction-mode unsupported platform/driver behavior.
tests/​Integration/​SearchPathPgbouncerTest.php	Adds PgBouncer transaction-pooling integration coverage for leakage prevention and edge cases.
phpunit.xml.dist	Splits unit vs integration test suites.
.github/​workflows/​ci.yaml	Adds PgBouncer services, runs PHPUnit with --fail-on-skipped, and posts coverage status.
docker-compose.yml	Adds local PgBouncer + Postgres setup for integration tests.
docker/​php/​Dockerfile	Adds a PHP image with pgsql extensions for local integration runs.
composer.json	Bumps PHPCS and adds a coverage-check tool for CI feedback.
README.md	Documents transaction mode usage, semantics, trade-offs, and limitations.

_{Review effort: Lite}

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues

• The new PostgreSQL 18 matrix entry doesn’t set coverage, so neither PHPUnit step runs… · New
• The new PostgreSQL 18 matrix entry doesn’t set coverage, so neither PHPUnit step runs… · New
• If COMMIT fails (e.g., serialization failure or error deferred to commit), the code rethrows… · New
• The docblock says calls are passed through unchanged when $isActive is false, but exec() still… · New
• The docblock says calls are passed through unchanged when $isActive is false, but exec() still… · New

5 resolved since last review

• Doctrine\\DBAL\\Driver\\Connection methods beginTransaction(), commit(), and rollBack() are…
• This integration test suite uses DIRECT_DATABASE_URL in multiple helpers…
• The boolean passed through the middleware is documented as “whether the transaction was opened by…
• Executing multiple statements in a single exec() call (BEGIN; SET LOCAL ...) is less portable…
• Granting pull-requests: write to the entire job broadens permissions for all steps (including…

_{Review effort: Lite}

[Copilot]

🔵 Needs a closer look

Critical transaction-control flaws can route queries to the wrong schema or prematurely commit caller-owned transactions.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Handle or reject transaction-control batches in Transaction mode

src/​Doctrine/​Middleware/​TransactionSearchPathConnection.php:202

A batch such as BEGIN; INSERT INTO marker (v) VALUES ('x'); COMMIT is classified as a single BEGIN operation and sent unchanged, so the write receives no SET LOCAL and targets the server's default or leaked schema. Transaction tracking also remains active after the batch commits. In Transaction mode, reject transaction-control batches or handle their individual statements with the appropriate schema application.

Reconcile transaction and schema state after failed raw COMMIT

src/​Doctrine/​Middleware/​TransactionSearchPathConnection.php:225

If a raw COMMIT fails on a deferred constraint, PostgreSQL ends the transaction, but the exception skips the state update here. The cached transaction and schema state then make the next query skip both implicit wrapping and SET LOCAL, so it runs against the pooled session's default or leaked schema. Reconcile transaction state and invalidate the schema cache on this error path, accounting for errors that leave a transaction open, and add a failed raw-commit integration test.

Track chained transactions after COMMIT or ROLLBACK AND CHAIN

src/​Doctrine/​Middleware/​TransactionSearchPathConnection.php:229

COMMIT AND CHAIN and ROLLBACK AND CHAIN leave a new PostgreSQL transaction open, but this branch marks the connection idle. The next schema-dependent statement gets an implicit wrapper whose COMMIT prematurely commits the caller's chained transaction, so a later rollback cannot undo its writes. Recognize chained endings, keep transaction tracking active while resetting the schema cache, and cover both forms with integration tests.

5 resolved since last review

• The new PostgreSQL 18 matrix entry doesn’t set coverage, so neither PHPUnit step runs…
• The new PostgreSQL 18 matrix entry doesn’t set coverage, so neither PHPUnit step runs…
• If COMMIT fails (e.g., serialization failure or error deferred to commit), the code rethrows…
• The docblock says calls are passed through unchanged when $isActive is false, but exec() still…
• The docblock says calls are passed through unchanged when $isActive is false, but exec() still…

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Human assessment is needed because transaction-control parsing gaps can allow cross-schema writes or unintentionally commit caller-owned transactions.

No open issues 🎉

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Critical gaps in transaction-control parsing and prepared-statement mode validation can bypass SET LOCAL protection and cause writes to the wrong schema.

2 open issues

• Nested comments bypass transaction-control detection · New
• Prepared statements bypass mode validation on execution · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Human assessment is needed because SQL parsing edge cases can bypass transaction tracking and allow writes to unintended schemas.

No open issues 🎉

2 resolved since last review

• Prepared statements bypass mode validation on execution
• Nested comments bypass transaction-control detection

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The regression test can miss a cross-schema write because the caught exception does not prove execution was prevented; it must also assert that public.marker contains no batch row.

1 open issue

• Verify rejected batches leave public.marker unchanged · New

_{Review effort: Balanced}

[github-actions]

[postgres-schema-bundle] (PHP 8.3, Symfony ^6.4) ✅ Total code coverage is 87.15 % – OK!
Well done, sailor! 🚣