Hi all, c5660e0 has introduced a new flag for MyXactFlags to restrict two-phase commit from working with temporary objects, and as a matter of fact XACT_FLAGS_ACCESSEDTEMPREL has been kept around to keep the error handling message compatible with past versions, still it is weird to keep both ACCESSEDTEMPNAMESPACE and ACCESSEDTEMPREL as the former implies the latter, so attached is a cleanup patch for HEAD.
Keeping both messages makes the error handling at PREPARE time perhaps a bit cleaner to make the difference about schema-level access or table-level access, still I'd rather simplify the code and just only keep the schema-level change as something we complain about. Another thing is that ACCESSEDTEMPREL is used in PreCommit_on_commit_actions() to avoid the truncates of ON COMMIT DELETE ROWS if no temporary tables have been accessed, still it would just mean that truncation would be tried on empty tables for nothing even if say a function is created in pg_temp. Thoughts? -- Michael
From 52c23f7a3f63dffa57d57ceeac7c1e2ef5ba68ad Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 16 Jan 2019 10:34:13 +0900
Subject: [PATCH] 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 18467d96d2..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 the temporary namespace
- * 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 d6d8f25141..b1dfe6499f 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, and make sure that non-parallel plans
--
2.20.1
signature.asc
Description: PGP signature
