From 227c7cf15602fa21af7eea1bd11375b7572649e3 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 21 Sep 2023 23:11:30 -0400 Subject: [PATCH] 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 12d768e70 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 --- src/backend/access/transam/xact.c | 1 + src/test/regress/expected/transactions.out | 40 ++++++++++++++++++++++++++++++ src/test/regress/sql/transactions.sql | 11 ++++++++ 3 files changed, 52 insertions(+) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 8daaa535ed..37c5e34cce 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -3036,6 +3036,7 @@ CommitTransactionCommand(void) TransactionState s = CurrentTransactionState; SavedTransactionCharacteristics savetc; + /* Must save in case we need to restore below */ SaveTransactionCharacteristics(&savetc); switch (s->blockState) diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 428c9edcc6..535f73cd40 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -853,6 +853,46 @@ SHOW transaction_deferrable; (1 row) COMMIT; +START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; + transaction_isolation +----------------------- + read committed +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +SAVEPOINT x; +COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT +SHOW transaction_isolation; + transaction_isolation +----------------------- + read committed +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +COMMIT; -- different mix of options just for fun START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; SHOW transaction_isolation; diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index 75ffe929d4..07db0d6ec6 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -489,6 +489,17 @@ SHOW transaction_read_only; SHOW transaction_deferrable; COMMIT; +START TRANSACTION ISOLATION LEVEL READ COMMITTED, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +SAVEPOINT x; +COMMIT AND CHAIN; -- TBLOCK_SUBCOMMIT +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +COMMIT; + -- different mix of options just for fun START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; SHOW transaction_isolation; -- 2.11.4.GIT