refactor(transaction): inherit postgres startTransaction

[abnegate]

Summary

• remove the redundant Postgres startTransaction() override
• let Postgres inherit the shared SQL::startTransaction() behavior
• assert Postgres now uses the shared SQL transaction path while retaining the desynced rollback regression coverage

Tests

• ./vendor/bin/phpunit tests/unit/SQLTransactionTest.php
• php -d memory_limit=2G ./vendor/bin/pint --test
• composer check
• git diff --check

Summary by CodeRabbit

Release Notes

• Refactor

  • Simplified transaction handling implementation for Postgres database connections.

• Tests

  • Updated transaction handling tests to validate correct behavior across database adapters.

[Copilot]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8312cc99-9d2e-479a-875e-4fae0f20bc86

📥 Commits

Reviewing files that changed from the base of the PR and between fff9f0e and a3ca087.

📒 Files selected for processing (2)

• src/Database/Adapter/Postgres.php
• tests/unit/SQLTransactionTest.php

💤 Files with no reviewable changes (1)

• src/Database/Adapter/Postgres.php

---

📝 Walkthrough

Walkthrough

The startTransaction(): bool override is removed from Postgres, causing the adapter to inherit the implementation from the parent SQL class. The test class is made final and gains a ReflectionMethod assertion confirming that Postgres::startTransaction is now declared on SQL, not Postgres.

Changes

Remove Postgres startTransaction override, inherit from SQL

Layer / File(s)	Summary
Remove Postgres::startTransaction and verify inheritance src/Database/Adapter/Postgres.php, tests/unit/SQLTransactionTest.php	Deletes the 38-line Postgres-specific startTransaction() implementation (inTransaction counter, beginTransaction(), SAVEPOINT transaction{n}, PDOException cleanup). Updates SQLTransactionTest to final and adds a ReflectionMethod assertion that Postgres::startTransaction's declaring class is SQL::class.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• utopia-php/database#677: Directly modifies the SQL::startTransaction() logic that Postgres now inherits after the override is removed.
• utopia-php/database#895: Introduces the desynced-rollback recovery behavior in SQL::startTransaction() that this PR now delegates to for Postgres.
• utopia-php/database#817: Modifies Postgres-specific nested transaction behavior in startTransaction()/rollbackTransaction(), directly preceding this removal.

Poem

🐇 Snip, snip, gone—those Postgres lines depart,
The parent SQL now plays the bigger part.
A SAVEPOINT lost, a method shed with care,
Reflection proves the child no longer declares.
Less code, more trust—the base class holds the art! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(transaction): inherit postgres startTransaction' directly and clearly describes the main change—removing a Postgres-specific override to inherit the shared SQL implementation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/remove-postgres-transaction-override

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR removes the startTransaction() override in Postgres and delegates to the shared SQL::startTransaction() implementation, which already contains identical logic (desynced-rollback cleanup, savepoint handling). The test suite is updated with a ReflectionMethod assertion to structurally enforce that Postgres no longer re-declares the method.

• Postgres.php: 44 lines of duplicate code removed; Postgres now inherits the shared SQL transaction path with no behavioral change.
• SQLTransactionTest.php: Class marked final; the existing desynced-rollback regression test is preserved and augmented with a reflection check that guards against the override being re-introduced accidentally.

Confidence Score: 5/5

Safe to merge — the change is a pure deduplication with no new behavioral differences introduced.

The removed Postgres override was structurally identical to SQL::startTransaction() in every way that matters at runtime (desynced-rollback cleanup, savepoint branching, exception wrapping). The reflection assertion in the test locks down the delegation going forward, and the desynced-rollback regression scenario continues to be exercised against a real Postgres adapter instance.

No files require special attention.

Important Files Changed

Filename	Overview
src/Database/Adapter/Postgres.php	Removes the redundant startTransaction() override, letting Postgres inherit SQL::startTransaction() directly. The removed code was functionally identical to the shared implementation except for an explicit $result boolean check on beginTransaction() — that concern is tracked in a previous thread.
tests/unit/SQLTransactionTest.php	Adds a ReflectionMethod assertion confirming Postgres::startTransaction is declared in SQL, not Postgres. The existing desynced-rollback test is kept and now serves as regression coverage for the shared path when used with a Postgres adapter.

_{Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/remove-post..." | Re-trigger Greptile}

[greptile-apps]

Preserve begin guard

Removing this override makes Postgres use the shared SQL::startTransaction() path. The old Postgres implementation guarded the boolean result from PDO::beginTransaction(), but the shared path ignores a false return, increments inTransaction, and reports success. If PDO fails this way, callers can run work believing a transaction is active when none was opened, and later commit or rollback state can be invalid. Please keep the boolean-result check in the shared implementation before removing this override.