From 883e60f9a369cd6eb140f054bb0b602ffa19c978 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 16 Jan 2019 10:34:13 +0900 Subject: [PATCH 2/2] Simplify 2PC restriction handling for temporary objects There are two flags used to control access to temporary tables and access to the temporary namespace of a session, however the first control flag is actually a concept included in the second. This removes the flag for temporary table tracking, keeping around only the one at namespace level. --- src/backend/access/heap/heapam.c | 4 ++-- src/backend/access/transam/xact.c | 19 ++++++------------- src/backend/commands/lockcmds.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/include/access/xact.h | 12 +++--------- src/test/regress/expected/temp.out | 14 +++++++------- 6 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 9afbc8228d..dd54810052 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -1144,7 +1144,7 @@ relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (RelationUsesLocalBuffers(r)) - MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL; + MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE; pgstat_initstats(r); @@ -1194,7 +1194,7 @@ try_relation_open(Oid relationId, LOCKMODE lockmode) /* Make note that we've accessed a temporary relation */ if (RelationUsesLocalBuffers(r)) - MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL; + MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE; pgstat_initstats(r); diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index aa61ad0df1..0c20729c68 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2259,13 +2259,18 @@ 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 accessed a temporary object 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. * + * Other objects types, like functions, operators or extensions, share the + * same restriction as they should not be created, locked or dropped as + * this can mess up with this session or even a follow-up session trying + * to use the same temporary namespace. + * * We must check this after executing any ON COMMIT actions, because they * might still access a temp relation. * @@ -2273,18 +2278,6 @@ PrepareTransaction(void) * cases, such as a temp table created and dropped all within the * transaction. That seems to require much more bookkeeping though. */ - if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot PREPARE a transaction that has operated on temporary tables"))); - - /* - * Similarly, PREPARE TRANSACTION is not allowed if the temporary - * namespace has been involved in this transaction as we cannot allow it - * to create, lock, or even drop objects within it as this can mess up - * with this session or even a follow-up session trying to use the same - * temporary namespace. - */ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 4cd7ec2d5f..bea99b59a2 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -108,7 +108,7 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid, */ relpersistence = get_rel_persistence(relid); if (relpersistence == RELPERSISTENCE_TEMP) - MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL; + MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE; /* Check permissions. */ aclresult = LockTableAclCheck(relid, lockmode, GetUserId()); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d2781cbf19..79b0a0d6e8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13253,7 +13253,7 @@ PreCommit_on_commit_actions(void) * relations, we can skip truncating ON COMMIT DELETE ROWS * tables, as they must still be empty. */ - if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL)) + if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPNAMESPACE)) oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid); break; case ONCOMMIT_DROP: diff --git a/src/include/access/xact.h b/src/include/access/xact.h index ed21a13896..ae89920687 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -87,10 +87,10 @@ extern int synchronous_commit; extern int MyXactFlags; /* - * XACT_FLAGS_ACCESSEDTEMPREL - set when a temporary relation is accessed. We - * don't allow PREPARE TRANSACTION in that case. + * XACT_FLAGS_ACCESSEDTEMPNAMESPACE - set when a temporary namespace is + * accessed. We don't allow PREPARE TRANSACTION in that case. */ -#define XACT_FLAGS_ACCESSEDTEMPREL (1U << 0) +#define XACT_FLAGS_ACCESSEDTEMPNAMESPACE (1U << 0) /* * XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK - records whether the top level xact @@ -98,12 +98,6 @@ extern int MyXactFlags; */ #define XACT_FLAGS_ACQUIREDACCESSEXCLUSIVELOCK (1U << 1) -/* - * XACT_FLAGS_ACCESSEDTEMPNAMESPACE - set when a temporary namespace is - * accessed. We don't allow PREPARE TRANSACTION in that case. - */ -#define XACT_FLAGS_ACCESSEDTEMPNAMESPACE (1U << 2) - /* * start- and end-of-transaction callbacks for dynamically loaded modules */ diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out index 860f58a3bf..d9e569a46d 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -327,15 +327,15 @@ ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; create type pg_temp.twophase_type as (a int); prepare transaction 'twophase_type'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; create view pg_temp.twophase_view as select 1; prepare transaction 'twophase_view'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; create sequence pg_temp.twophase_seq; prepare transaction 'twophase_sequence'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace -- Temporary tables cannot be used with two-phase commit. create temp table twophase_tab (a int); begin; @@ -345,19 +345,19 @@ select a from twophase_tab; (0 rows) prepare transaction 'twophase_tab'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; insert into twophase_tab values (1); prepare transaction 'twophase_tab'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; lock twophase_tab in access exclusive mode; prepare transaction 'twophase_tab'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace begin; drop table twophase_tab; prepare transaction 'twophase_tab'; -ERROR: cannot PREPARE a transaction that has operated on temporary tables +ERROR: cannot PREPARE a transaction that has operated on temporary namespace -- Corner case: current_schema may create a temporary schema if namespace -- creation is pending, so check after that. First reset the connection -- to remove the temporary namespace. -- 2.20.1