commit 0df9d036f3fd9b8a8e22b0101b4632b5a8f62ea0 Author: Stas Kelvich Date: Tue May 14 12:47:59 2019 +0300 Read-only access to temp tables for 2PC transactions diff --git a/src/backend/access/common/relation.c b/src/backend/access/common/relation.c index 41a0051f88..9e35311ad4 100644 --- a/src/backend/access/common/relation.c +++ b/src/backend/access/common/relation.c @@ -23,6 +23,7 @@ #include "access/relation.h" #include "access/xact.h" #include "catalog/namespace.h" +#include "commands/tablecmds.h" #include "miscadmin.h" #include "pgstat.h" #include "storage/lmgr.h" @@ -69,9 +70,18 @@ relation_open(Oid relationId, LOCKMODE lockmode) IsBootstrapProcessingMode() || CheckRelationLockedByMe(r, AccessShareLock, true)); - /* Make note that we've accessed a temporary relation */ - if (RelationUsesLocalBuffers(r)) + /* + * Make note that we've accessed a temporary relation with at least + * RowShareLock lockmode. See also comments above MyXactFlags check + * in PrepareTransaction(). + */ + if (RelationUsesLocalBuffers(r) && + (lockmode > AccessShareLock || + (lockmode == NoLock && + CheckRelationLockedByMe(r, RowShareLock, true)))) + { MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE; + } pgstat_initstats(r); @@ -119,9 +129,18 @@ try_relation_open(Oid relationId, LOCKMODE lockmode) Assert(lockmode != NoLock || CheckRelationLockedByMe(r, AccessShareLock, true)); - /* Make note that we've accessed a temporary relation */ - if (RelationUsesLocalBuffers(r)) + /* + * Make note that we've accessed a temporary relation with at least + * RowShareLock lockmode. See also comments above MyXactFlags check + * in PrepareTransaction(). + */ + if (RelationUsesLocalBuffers(r) && + (lockmode > AccessShareLock || + (lockmode == NoLock && + CheckRelationLockedByMe(r, RowShareLock, true)))) + { MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE; + } pgstat_initstats(r); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 20feeec327..39a4c1106d 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2352,12 +2352,14 @@ PrepareTransaction(void) /* NOTIFY will be handled below */ /* - * Don't allow PREPARE TRANSACTION if we've accessed a temporary table in + * Don't allow PREPARE TRANSACTION if we've modified a temporary table in * this transaction. Having the prepared xact hold locks on another * backend's temp table seems a bad idea --- for instance it would prevent * the backend from exiting. There are other problems too, such as how to * clean up the source backend's local buffers and ON COMMIT state if the - * prepared xact includes a DROP of a temp table. + * prepared xact includes a DROP of a temp table. Special case of reading + * from a temporary table is allowed by dropping AccessShare locks during + * PREPARE. * * Other objects types, like functions, operators or extensions, share the * same restriction as they should not be created, locked or dropped as @@ -2368,8 +2370,9 @@ PrepareTransaction(void) * might still access a temp relation. * * XXX In principle this could be relaxed to allow some useful special - * cases, such as a temp table created and dropped all within the - * transaction. That seems to require much more bookkeeping though. + * cases besides reading from a temp table, such as a temp table created + * and dropped all within the transaction. That seems to require much more + * bookkeeping though. */ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE)) ereport(ERROR, diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index c8958766f1..9eee017c3d 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -3212,6 +3212,13 @@ AtPrepare_Locks(void) locallock->lock = locallock->proclock->tag.myLock; } + /* + * Do not save any AccessShare locks as they will not be transfered to + * a dummy PGPRPROC by PostPrepare_Locks(). + */ + if (locallock->tag.mode == AccessShareLock) + continue; + /* * Arrange to not release any strong lock count held by this lock * entry. We must retain the count until the prepared transaction is @@ -3382,6 +3389,33 @@ PostPrepare_Locks(TransactionId xid) if (proclock->releaseMask != proclock->holdMask) elog(PANIC, "we seem to have dropped a bit somewhere"); + /* + * To be able to safely read from temp tables prepared transaction + * should not hold AccessShare locks. So here we are releasing any + * AccessShare locks without transfering them further to a dummy + * PGPROC. See also comments above MyXactFlags check in + * PrepareTransaction(). + */ + if (proclock->holdMask & LOCKBIT_ON(AccessShareLock)) + { + LockMethod lockMethodTable; + bool wakeupNeeded; + + lockMethodTable = LockMethods[lock->tag.locktag_lockmethodid]; + wakeupNeeded = UnGrantLock(lock, AccessShareLock, + proclock, lockMethodTable); + + CleanUpLock(lock, proclock, lockMethodTable, + LockTagHashCode(&lock->tag), wakeupNeeded); + + /* + * If we were holding lock only in AccessShare mode than + * there is nothing to pass to a new PGPROC. + */ + if (proclock->holdMask == 0) + continue; + } + /* * We cannot simply modify proclock->tag.myProc to reassign * ownership of the lock, because that's part of the hash key and diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out index eb77c18788..14ea813342 100644 --- a/src/test/regress/expected/prepared_xacts.out +++ b/src/test/regress/expected/prepared_xacts.out @@ -241,6 +241,36 @@ SELECT * FROM pxtest3; ERROR: relation "pxtest3" does not exist LINE 1: SELECT * FROM pxtest3; ^ +-- Read-only access to temporary table is allowed from a prepared +-- transaction +create temp table twophase_tab_preparable (a int); +begin; +select a from twophase_tab_preparable; + a +--- +(0 rows) + +prepare transaction 'twophase_tab_preparable'; +-- prepared transactions should not hold AccessShare locks +create table twophase_tab_testdrop1 (a int); +create table twophase_tab_testdrop2 (a int); +begin; +select a from twophase_tab_testdrop1; + a +--- +(0 rows) + +select a from twophase_tab_testdrop2; + a +--- +(0 rows) + +prepare transaction 'twophase_tab_testdrop'; +drop table twophase_tab_testdrop1; +\c - +drop table twophase_tab_testdrop2; +commit prepared 'twophase_tab_preparable'; +commit prepared 'twophase_tab_testdrop'; -- There should be no prepared transactions SELECT gid FROM pg_prepared_xacts; gid diff --git a/src/test/regress/expected/prepared_xacts_1.out b/src/test/regress/expected/prepared_xacts_1.out index 0857d259e0..b7f0f56818 100644 --- a/src/test/regress/expected/prepared_xacts_1.out +++ b/src/test/regress/expected/prepared_xacts_1.out @@ -235,6 +235,42 @@ SELECT * FROM pxtest3; ----- (0 rows) +-- Read-only access to temporary table is allowed from a prepared +-- transaction +create temp table twophase_tab_preparable (a int); +begin; +select a from twophase_tab_preparable; + a +--- +(0 rows) + +prepare transaction 'twophase_tab_preparable'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +-- prepared transactions should not hold AccessShare locks +create table twophase_tab_testdrop1 (a int); +create table twophase_tab_testdrop2 (a int); +begin; +select a from twophase_tab_testdrop1; + a +--- +(0 rows) + +select a from twophase_tab_testdrop2; + a +--- +(0 rows) + +prepare transaction 'twophase_tab_testdrop'; +ERROR: prepared transactions are disabled +HINT: Set max_prepared_transactions to a nonzero value. +drop table twophase_tab_testdrop1; +\c - +drop table twophase_tab_testdrop2; +commit prepared 'twophase_tab_preparable'; +ERROR: prepared transaction with identifier "twophase_tab_preparable" does not exist +commit prepared 'twophase_tab_testdrop'; +ERROR: prepared transaction with identifier "twophase_tab_testdrop" does not exist -- There should be no prepared transactions SELECT gid FROM pg_prepared_xacts; gid diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out index ad7d558191..1f4a342f75 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -336,17 +336,9 @@ begin; create sequence pg_temp.twophase_seq; prepare transaction 'twophase_sequence'; ERROR: cannot PREPARE a transaction that has operated on temporary objects --- Temporary tables cannot be used with two-phase commit. +-- Temporary tables cannot be modified by prepared transaction. create temp table twophase_tab (a int); begin; -select a from twophase_tab; - a ---- -(0 rows) - -prepare transaction 'twophase_tab'; -ERROR: cannot PREPARE a transaction that has operated on temporary objects -begin; insert into twophase_tab values (1); prepare transaction 'twophase_tab'; ERROR: cannot PREPARE a transaction that has operated on temporary objects diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql index d8249a27dc..59c5435239 100644 --- a/src/test/regress/sql/prepared_xacts.sql +++ b/src/test/regress/sql/prepared_xacts.sql @@ -149,6 +149,26 @@ SELECT gid FROM pg_prepared_xacts; COMMIT PREPARED 'regress-two'; SELECT * FROM pxtest3; +-- Read-only access to temporary table is allowed from a prepared +-- transaction +create temp table twophase_tab_preparable (a int); +begin; +select a from twophase_tab_preparable; +prepare transaction 'twophase_tab_preparable'; + +-- prepared transactions should not hold AccessShare locks +create table twophase_tab_testdrop1 (a int); +create table twophase_tab_testdrop2 (a int); +begin; +select a from twophase_tab_testdrop1; +select a from twophase_tab_testdrop2; +prepare transaction 'twophase_tab_testdrop'; +drop table twophase_tab_testdrop1; +\c - +drop table twophase_tab_testdrop2; +commit prepared 'twophase_tab_preparable'; +commit prepared 'twophase_tab_testdrop'; + -- There should be no prepared transactions SELECT gid FROM pg_prepared_xacts; diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql index e634ddb9ca..4643278d84 100644 --- a/src/test/regress/sql/temp.sql +++ b/src/test/regress/sql/temp.sql @@ -257,12 +257,9 @@ begin; create sequence pg_temp.twophase_seq; prepare transaction 'twophase_sequence'; --- Temporary tables cannot be used with two-phase commit. +-- Temporary tables cannot be modified by prepared transaction. create temp table twophase_tab (a int); begin; -select a from twophase_tab; -prepare transaction 'twophase_tab'; -begin; insert into twophase_tab values (1); prepare transaction 'twophase_tab'; begin;