Skip to content

refactor(transaction): inherit postgres startTransaction #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abnegate merged 2 commits into from
Jun 22, 2026
+6 −39
Merged

refactor(transaction): inherit postgres startTransaction #897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49dd98e
refactor(transaction): inherit postgres startTransaction
abnegate Jun 18, 2026
a3ca087
Merge branch 'main' into fix/remove-postgres-transaction-override
abnegate Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 0 additions & 38 deletions src/Database/Adapter/Postgres.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,44 +31,6 @@
class Postgres extends SQL
{
public const MAX_IDENTIFIER_NAME = 63;
Comment on lines 31 to 33

@greptile-apps greptile-apps Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

/**
* @inheritDoc
*/
public function startTransaction(): bool
{
try {
if ($this->inTransaction === 0) {
try {
if ($this->getPDO()->inTransaction()) {
$this->getPDO()->rollBack();
} else {
// If no active transaction, this has no effect.
$this->getPDO()->prepare('ROLLBACK')->execute();
}
} catch (PDOException) {
// A pooled connection can report a transaction it no longer
// holds after a reconnect (e.g. Swoole PDOProxy keeps its own
// counter), making this cleanup rollback throw. It is best
// effort; swallow it and begin a fresh transaction below.
}

$result = $this->getPDO()->beginTransaction();
} else {
$this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction);
$result = true;
}
} catch (PDOException $e) {
throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e);
}

if (!$result) {
throw new TransactionException('Failed to start transaction');
}

$this->inTransaction++;

return $result;
}

/**
* @inheritDoc
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/SQLTransactionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

use PDOException;
use PHPUnit\Framework\TestCase;
use ReflectionMethod;
use Utopia\Database\Adapter\MySQL;
use Utopia\Database\Adapter\Postgres;
use Utopia\Database\Adapter\SQL;
use Utopia\Database\Exception\Transaction as TransactionException;

class SQLTransactionTest extends TestCase
final class SQLTransactionTest extends TestCase
{
/**
* A pooled connection (e.g. Swoole PDOProxy) can keep its own transaction
Expand Down Expand Up @@ -61,6 +63,9 @@ public function testStartTransactionDoesNotMaskBeginFailureAfterDesyncedRollback

public function testPostgresStartTransactionRecoversFromDesyncedRollback(): void
{
$method = new ReflectionMethod(Postgres::class, 'startTransaction');
$this->assertSame(SQL::class, $method->getDeclaringClass()->getName());

$pdo = $this->getMockBuilder(\PDO::class)
->disableOriginalConstructor()
->getMock();
Expand Down
Loading