On Mon, Jan 21, 2019 at 02:58:39PM -0300, Alvaro Herrera wrote: > "... operated on temp namespace" doesn't look good; seems to me to be > missing an article, for one thing, but really I'm not sure that > 'namespace' is the term to be using here. I'd say "... operated on > temporary objects" instead (the plural avoids the need for the article; > and the user doesn't really care about the namespace itself but rather > about the objects it contains.)
Thanks for the input. Would you prefer something like the attached then? I have switched the error message to "temporary objects", and renamed the flag to XACT_FLAGS_ACCESSEDTEMPOBJECTS. -- Michael
From bd18d3c8a683a11d0fc89194cc0c60672e62b0e1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 22 Jan 2019 09:30:01 +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, renaming it for all kinds of temporary objects.
---
src/backend/access/heap/heapam.c | 4 ++--
src/backend/access/transam/xact.c | 23 +++++++------------
src/backend/catalog/namespace.c | 2 +-
src/backend/commands/dropcmds.c | 2 +-
src/backend/commands/extension.c | 2 +-
src/backend/commands/lockcmds.c | 2 +-
src/backend/commands/tablecmds.c | 2 +-
src/include/access/xact.h | 12 +++-------
.../expected/test_extensions.out | 2 +-
src/test/regress/expected/temp.out | 22 +++++++++---------
10 files changed, 30 insertions(+), 43 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9afbc8228d..205b37e61f 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_ACCESSEDTEMPOBJECTS;
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_ACCESSEDTEMPOBJECTS;
pgstat_initstats(r);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 18467d96d2..8525cc5f8e 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,22 +2278,10 @@ 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))
+ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPOBJECTS))
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),
- errmsg("cannot PREPARE a transaction that has operated on temporary namespace")));
+ errmsg("cannot PREPARE a transaction that has operated on temporary objects")));
/*
* Likewise, don't allow PREPARE after pg_export_snapshot. This could be
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index cdd5006a72..1440423d94 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3844,7 +3844,7 @@ AccessTempTableNamespace(bool force)
* Make note that this temporary namespace has been accessed in this
* transaction.
*/
- MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+ MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPOBJECTS;
/*
* If the caller attempting to access a temporary schema expects the
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index 980ce89c62..3ec8a8cdc8 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -113,7 +113,7 @@ RemoveObjects(DropStmt *stmt)
* transaction.
*/
if (OidIsValid(namespaceId) && isTempNamespace(namespaceId))
- MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+ MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPOBJECTS;
/* Release any relcache reference count, but keep lock until commit. */
if (relation)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 4fe71196c4..70cec39364 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1480,7 +1480,7 @@ CreateExtensionInternal(char *extensionName,
* transaction.
*/
if (isTempNamespace(schemaOid))
- MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+ MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPOBJECTS;
/*
* We don't check creation rights on the target namespace here. If the
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 4cd7ec2d5f..15166f88b5 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_ACCESSEDTEMPOBJECTS;
/* Check permissions. */
aclresult = LockTableAclCheck(relid, lockmode, GetUserId());
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a1ac698e5..b2f05e7b28 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13578,7 +13578,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_ACCESSEDTEMPOBJECTS))
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..adcfad9d4b 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_ACCESSEDTEMPOBJECTS - set when a temporary object is accessed.
+ * We don't allow PREPARE TRANSACTION in that case.
*/
-#define XACT_FLAGS_ACCESSEDTEMPREL (1U << 0)
+#define XACT_FLAGS_ACCESSEDTEMPOBJECTS (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/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index 1eec5a37d3..b5cbdfcad4 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -148,7 +148,7 @@ SELECT create_extension_with_temp_schema();
(1 row)
PREPARE TRANSACTION 'twophase_extension';
-ERROR: cannot PREPARE a transaction that has operated on temporary namespace
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
-- Clean up
DROP TABLE test_ext4_tab;
DROP FUNCTION create_extension_with_temp_schema();
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index d6d8f25141..056e5ecf33 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -310,32 +310,32 @@ begin;
create function pg_temp.twophase_func() returns void as
$$ select '2pc_func'::text $$ language sql;
prepare transaction 'twophase_func';
-ERROR: cannot PREPARE a transaction that has operated on temporary namespace
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
-- Function drop
create function pg_temp.twophase_func() returns void as
$$ select '2pc_func'::text $$ language sql;
begin;
drop function pg_temp.twophase_func();
prepare transaction 'twophase_func';
-ERROR: cannot PREPARE a transaction that has operated on temporary namespace
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
-- Operator creation
begin;
create operator pg_temp.@@ (leftarg = int4, rightarg = int4, procedure = int4mi);
prepare transaction 'twophase_operator';
-ERROR: cannot PREPARE a transaction that has operated on temporary namespace
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
-- These generate errors about temporary tables.
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 objects
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 objects
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 objects
-- 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 objects
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 objects
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 objects
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 objects
-- 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
@@ -374,4 +374,4 @@ SELECT current_schema() ~ 'pg_temp' AS is_temp_schema;
(1 row)
PREPARE TRANSACTION 'twophase_search';
-ERROR: cannot PREPARE a transaction that has operated on temporary namespace
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
--
2.20.1
signature.asc
Description: PGP signature
