Skip to content

Commit 48e2b23

Browse files
tglsfdctglsfdc
committed
Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.
In older branches, COMMIT/ROLLBACK AND CHAIN failed to propagate the current transaction's properties to the new transaction if there was any open subtransaction (unreleased savepoint). Instead, some previous transaction's properties would be restored. This is because the "if (s->chain)" check in CommitTransactionCommand examined the wrong instance of the "chain" flag and falsely concluded that it didn't need to save transaction properties. Our regression tests would have noticed this, except they used identical transaction properties for multiple tests in a row, so that the faulty behavior was not distinguishable from correct behavior. Commit 12d768e fixed the problem in v15 and later, but only rather accidentally, because I removed the "if (s->chain)" test to avoid a compiler warning, while not realizing that the warning was flagging a real bug. In v14 and before, remove the if-test and save transaction properties unconditionally; just as in the newer branches, that's not expensive enough to justify thinking harder. Add the comment and extra regression test to v15 and later to forestall any future recurrence, but there's no live bug in those branches. Patch by me, per bug #18118 from Liu Xiang. Back-patch to v12 where the AND CHAIN feature was added. Discussion: https://postgr.es/m/18118-4b72fcbb903aace6@postgresql.org
1 parent cca97ce commit 48e2b23

File tree

3 files changed

+52
-0
lines changed

3 files changed

+52
-0
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3036,6 +3036,7 @@ CommitTransactionCommand(void)
30363036
TransactionState s = CurrentTransactionState;
30373037
SavedTransactionCharacteristics savetc;
30383038

3039+
/* Must save in case we need to restore below */
30393040
SaveTransactionCharacteristics(&savetc);
30403041

30413042
switch (s->blockState)

‎src/test/regress/expected/transactions.out

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,46 @@ SHOW transaction_deferrable;
852852
on
853853
(1 row)
854854

855+
COMMIT;
856+
START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE;
857+
SHOW transaction_isolation;
858+
transaction_isolation
859+
-----------------------
860+
read committed
861+
(1 row)
862+
863+
SHOW transaction_read_only;
864+
transaction_read_only
865+
-----------------------
866+
off
867+
(1 row)
868+
869+
SHOW transaction_deferrable;
870+
transaction_deferrable
871+
------------------------
872+
on
873+
(1 row)
874+
875+
SAVEPOINT x;
876+
COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT
877+
SHOW transaction_isolation;
878+
transaction_isolation
879+
-----------------------
880+
read committed
881+
(1 row)
882+
883+
SHOW transaction_read_only;
884+
transaction_read_only
885+
-----------------------
886+
off
887+
(1 row)
888+
889+
SHOW transaction_deferrable;
890+
transaction_deferrable
891+
------------------------
892+
on
893+
(1 row)
894+
855895
COMMIT;
856896
-- different mix of options just for fun
857897
START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE;

‎src/test/regress/sql/transactions.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,17 @@ SHOW transaction_read_only;
489489
SHOW transaction_deferrable;
490490
COMMIT;
491491

492+
START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE;
493+
SHOW transaction_isolation;
494+
SHOW transaction_read_only;
495+
SHOW transaction_deferrable;
496+
SAVEPOINT x;
497+
COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT
498+
SHOW transaction_isolation;
499+
SHOW transaction_read_only;
500+
SHOW transaction_deferrable;
501+
COMMIT;
502+
492503
-- different mix of options just for fun
493504
START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE;
494505
SHOW transaction_isolation;

0 commit comments

Comments
 (0)