From eb97099e0d2a6e01a219cbdaa1e14ab0f13620d7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 16 Jan 2019 10:14:08 +0900
Subject: [PATCH 1/2] Restrict more the use of temporary namespace in two-phase
 transactions

Attempting to use a temporary table within a two phase transaction is
forbidden for ages.  However, there have been uncovered grounds for
a couple of other object types and commands which work on temporary
objects with two-phase commit.  In short, trying to create, lock or drop
an object on a temporary schema should not be authorized within a
two-phase transaction, as it would cause its state to create
dependencies with other sessions, causing all sorts of side effects with
the existing session or other sessions spawned later on trying to use
the same temporary schema name.

Regression tests are added to cover all the grounds found, the original
report mentioned function creation, but monitoring closer there are many
other patterns like LOCK, DROP or CREATE EXTENSION which are involved.

This is back-patched down to v10, which is where 9b013dc has introduced
MyXactFlags, something that this patch relies on.

Reported-by: Alexey Bashtanov
Author: Michael Paquier
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/5d910e2e-0db8-ec06-dd5f-baec420513c3@imap.cc
Backpatch-through: 10
---
 doc/src/sgml/ref/prepare_transaction.sgml     |  6 +-
 src/backend/access/transam/xact.c             | 12 ++++
 src/backend/catalog/namespace.c               | 60 ++++++++++++----
 src/backend/commands/dropcmds.c               |  8 +++
 src/backend/commands/extension.c              |  7 ++
 src/backend/commands/lockcmds.c               | 10 +++
 src/include/access/xact.h                     |  5 ++
 .../expected/test_extensions.out              | 33 +++++++++
 .../test_extensions/sql/test_extensions.sql   | 29 ++++++++
 src/test/regress/expected/temp.out            | 71 +++++++++++++++++++
 src/test/regress/sql/temp.sql                 | 56 +++++++++++++++
 11 files changed, 279 insertions(+), 18 deletions(-)

diff --git a/doc/src/sgml/ref/prepare_transaction.sgml b/doc/src/sgml/ref/prepare_transaction.sgml
index d958f7a06f..5016ca287e 100644
--- a/doc/src/sgml/ref/prepare_transaction.sgml
+++ b/doc/src/sgml/ref/prepare_transaction.sgml
@@ -98,9 +98,9 @@ PREPARE TRANSACTION <replaceable class="parameter">transaction_id</replaceable>
 
   <para>
    It is not currently allowed to <command>PREPARE</command> a transaction that
-   has executed any operations involving temporary tables,
-   created any cursors <literal>WITH HOLD</literal>, or executed
-   <command>LISTEN</command>, <command>UNLISTEN</command>, or
+   has executed any operations involving temporary tables or the session's
+   temporary namespace, created any cursors <literal>WITH HOLD</literal>, or
+   executed <command>LISTEN</command>, <command>UNLISTEN</command>, or
    <command>NOTIFY</command>.
    Those features are too tightly
    tied to the current session to be useful in a transaction to be prepared.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index f665e38ecf..aa61ad0df1 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2278,6 +2278,18 @@ PrepareTransaction(void)
 				(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),
+				 errmsg("cannot PREPARE a transaction that has operated on temporary namespace")));
+
 	/*
 	 * Likewise, don't allow PREPARE after pg_export_snapshot.  This could be
 	 * supported if we added cleanup logic to twophase.c, but for now it
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 5a6eb495d4..a6be183095 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -192,6 +192,7 @@ char	   *namespace_search_path = NULL;
 
 /* Local functions */
 static void recomputeNamespacePath(void);
+static void AccessTempTableNamespace(bool force);
 static void InitTempTableNamespace(void);
 static void RemoveTempRelations(Oid tempNamespaceId);
 static void RemoveTempRelationsCallback(int code, Datum arg);
@@ -459,9 +460,8 @@ RangeVarGetCreationNamespace(const RangeVar *newRelation)
 		/* check for pg_temp alias */
 		if (strcmp(newRelation->schemaname, "pg_temp") == 0)
 		{
-			/* Initialize temp namespace if first time through */
-			if (!OidIsValid(myTempNamespace))
-				InitTempTableNamespace();
+			/* Initialize temp namespace */
+			AccessTempTableNamespace(false);
 			return myTempNamespace;
 		}
 		/* use exact schema given */
@@ -470,9 +470,8 @@ RangeVarGetCreationNamespace(const RangeVar *newRelation)
 	}
 	else if (newRelation->relpersistence == RELPERSISTENCE_TEMP)
 	{
-		/* Initialize temp namespace if first time through */
-		if (!OidIsValid(myTempNamespace))
-			InitTempTableNamespace();
+		/* Initialize temp namespace */
+		AccessTempTableNamespace(false);
 		return myTempNamespace;
 	}
 	else
@@ -482,7 +481,7 @@ RangeVarGetCreationNamespace(const RangeVar *newRelation)
 		if (activeTempCreationPending)
 		{
 			/* Need to initialize temp namespace */
-			InitTempTableNamespace();
+			AccessTempTableNamespace(true);
 			return myTempNamespace;
 		}
 		namespaceId = activeCreationNamespace;
@@ -2921,9 +2920,8 @@ LookupCreationNamespace(const char *nspname)
 	/* check for pg_temp alias */
 	if (strcmp(nspname, "pg_temp") == 0)
 	{
-		/* Initialize temp namespace if first time through */
-		if (!OidIsValid(myTempNamespace))
-			InitTempTableNamespace();
+		/* Initialize temp namespace */
+		AccessTempTableNamespace(false);
 		return myTempNamespace;
 	}
 
@@ -2986,9 +2984,8 @@ QualifiedNameGetCreationNamespace(List *names, char **objname_p)
 		/* check for pg_temp alias */
 		if (strcmp(schemaname, "pg_temp") == 0)
 		{
-			/* Initialize temp namespace if first time through */
-			if (!OidIsValid(myTempNamespace))
-				InitTempTableNamespace();
+			/* Initialize temp namespace */
+			AccessTempTableNamespace(false);
 			return myTempNamespace;
 		}
 		/* use exact schema given */
@@ -3002,7 +2999,7 @@ QualifiedNameGetCreationNamespace(List *names, char **objname_p)
 		if (activeTempCreationPending)
 		{
 			/* Need to initialize temp namespace */
-			InitTempTableNamespace();
+			AccessTempTableNamespace(true);
 			return myTempNamespace;
 		}
 		namespaceId = activeCreationNamespace;
@@ -3832,6 +3829,39 @@ recomputeNamespacePath(void)
 	list_free(oidlist);
 }
 
+/*
+ * AccessTempTableNamespace
+ *		Provide access to a temporary namespace, potentially creating it
+ *		if not present yet.  This routine registers if the namespace gets
+ *		in use in this transaction.  'force' can be set to true to allow
+ *		the caller to enforce the creation of the temporary namespace for
+ *		use in this backend, which happens for one whose creation is
+ *		pending.
+ */
+static void
+AccessTempTableNamespace(bool force)
+{
+	/*
+	 * Make note that this temporary namespace has been accessed in this
+	 * transaction.
+	 */
+	MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+
+	/*
+	 * If the caller attempting to access a temporary schema expects the
+	 * creation of the namespace to be pending and should be enforced, then go
+	 * through the creation.
+	 */
+	if (!force && OidIsValid(myTempNamespace))
+		return;
+
+	/*
+	 * The temporary tablespace does not exist yet and is wanted, so
+	 * initialize it.
+	 */
+	InitTempTableNamespace();
+}
+
 /*
  * InitTempTableNamespace
  *		Initialize temp table namespace on first use in a particular backend
@@ -4275,7 +4305,7 @@ fetch_search_path(bool includeImplicit)
 	 */
 	if (activeTempCreationPending)
 	{
-		InitTempTableNamespace();
+		AccessTempTableNamespace(true);
 		recomputeNamespacePath();
 	}
 
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index 1dc1c746f1..980ce89c62 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "access/xact.h"
 #include "access/heapam.h"
 #include "access/htup_details.h"
 #include "catalog/dependency.h"
@@ -107,6 +108,13 @@ RemoveObjects(DropStmt *stmt)
 			check_object_ownership(GetUserId(), stmt->removeType, address,
 								   object, relation);
 
+		/*
+		 * Make note if a temporary namespace has been accessed in this
+		 * transaction.
+		 */
+		if (OidIsValid(namespaceId) && isTempNamespace(namespaceId))
+			MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+
 		/* Release any relcache reference count, but keep lock until commit. */
 		if (relation)
 			heap_close(relation, NoLock);
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index e381a30760..4fe71196c4 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1475,6 +1475,13 @@ CreateExtensionInternal(char *extensionName,
 		list_free(search_path);
 	}
 
+	/*
+	 * Make note if a temporary namespace has been accessed in this
+	 * transaction.
+	 */
+	if (isTempNamespace(schemaOid))
+		MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPNAMESPACE;
+
 	/*
 	 * We don't check creation rights on the target namespace here.  If the
 	 * extension script actually creates any objects there, it will fail if
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 1b33be1895..4cd7ec2d5f 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "access/xact.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_inherits.h"
 #include "commands/lockcmds.h"
@@ -83,6 +84,7 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
 {
 	LOCKMODE	lockmode = *(LOCKMODE *) arg;
 	char		relkind;
+	char		relpersistence;
 	AclResult	aclresult;
 
 	if (!OidIsValid(relid))
@@ -100,6 +102,14 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
 				 errmsg("\"%s\" is not a table or a view",
 						rv->relname)));
 
+	/*
+	 * Make note if a temporary relation has been accessed in this
+	 * transaction.
+	 */
+	relpersistence = get_rel_persistence(relid);
+	if (relpersistence == RELPERSISTENCE_TEMP)
+		MyXactFlags |= XACT_FLAGS_ACCESSEDTEMPREL;
+
 	/* Check permissions. */
 	aclresult = LockTableAclCheck(relid, lockmode, GetUserId());
 	if (aclresult != ACLCHECK_OK)
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
index 169cf2834c..ed21a13896 100644
--- a/src/include/access/xact.h
+++ b/src/include/access/xact.h
@@ -98,6 +98,11 @@ 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 28d86c4b87..1eec5a37d3 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -121,3 +121,36 @@ Objects in extension "test_ext8"
 
 -- dropping it should still work
 drop extension test_ext8;
+-- Test creation of extension in temporary schema with two-phase commit,
+-- which should not work.  This function wrapper is useful for portability.
+-- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
+-- schema name.
+\set SHOW_CONTEXT never
+SET client_min_messages TO 'warning';
+-- First enforce presence of temporary schema.
+CREATE TEMP TABLE test_ext4_tab ();
+CREATE OR REPLACE FUNCTION create_extension_with_temp_schema()
+  RETURNS VOID AS $$
+  DECLARE
+    tmpschema text;
+    query text;
+  BEGIN
+    SELECT INTO tmpschema pg_my_temp_schema()::regnamespace;
+    query := 'CREATE EXTENSION test_ext4 SCHEMA ' || tmpschema || ' CASCADE;';
+    RAISE NOTICE 'query %', query;
+    EXECUTE query;
+  END; $$ LANGUAGE plpgsql;
+BEGIN;
+SELECT create_extension_with_temp_schema();
+ create_extension_with_temp_schema 
+-----------------------------------
+ 
+(1 row)
+
+PREPARE TRANSACTION 'twophase_extension';
+ERROR:  cannot PREPARE a transaction that has operated on temporary namespace
+-- Clean up
+DROP TABLE test_ext4_tab;
+DROP FUNCTION create_extension_with_temp_schema();
+RESET client_min_messages;
+\unset SHOW_CONTEXT
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 9e64503eb5..f505466ab4 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -64,3 +64,32 @@ end';
 
 -- dropping it should still work
 drop extension test_ext8;
+
+-- Test creation of extension in temporary schema with two-phase commit,
+-- which should not work.  This function wrapper is useful for portability.
+
+-- Avoid noise caused by CONTEXT and NOTICE messages including the temporary
+-- schema name.
+\set SHOW_CONTEXT never
+SET client_min_messages TO 'warning';
+-- First enforce presence of temporary schema.
+CREATE TEMP TABLE test_ext4_tab ();
+CREATE OR REPLACE FUNCTION create_extension_with_temp_schema()
+  RETURNS VOID AS $$
+  DECLARE
+    tmpschema text;
+    query text;
+  BEGIN
+    SELECT INTO tmpschema pg_my_temp_schema()::regnamespace;
+    query := 'CREATE EXTENSION test_ext4 SCHEMA ' || tmpschema || ' CASCADE;';
+    RAISE NOTICE 'query %', query;
+    EXECUTE query;
+  END; $$ LANGUAGE plpgsql;
+BEGIN;
+SELECT create_extension_with_temp_schema();
+PREPARE TRANSACTION 'twophase_extension';
+-- Clean up
+DROP TABLE test_ext4_tab;
+DROP FUNCTION create_extension_with_temp_schema();
+RESET client_min_messages;
+\unset SHOW_CONTEXT
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index f018f17ca0..860f58a3bf 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -301,3 +301,74 @@ select relname from pg_class where relname like 'temp_inh_oncommit_test%';
 (1 row)
 
 drop table temp_inh_oncommit_test;
+-- Tests with two-phase commit
+-- Transactions creating objects in a temporary namespace cannot be used
+-- with two-phase commit.
+-- These cases generate errors about temporary namespace.
+-- Function creation
+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
+-- 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
+-- 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
+-- 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
+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
+begin;
+create sequence pg_temp.twophase_seq;
+prepare transaction 'twophase_sequence';
+ERROR:  cannot PREPARE a transaction that has operated on temporary tables
+-- Temporary tables cannot be used with two-phase commit.
+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 tables
+begin;
+insert into twophase_tab values (1);
+prepare transaction 'twophase_tab';
+ERROR:  cannot PREPARE a transaction that has operated on temporary tables
+begin;
+lock twophase_tab in access exclusive mode;
+prepare transaction 'twophase_tab';
+ERROR:  cannot PREPARE a transaction that has operated on temporary tables
+begin;
+drop table twophase_tab;
+prepare transaction 'twophase_tab';
+ERROR:  cannot PREPARE a transaction that has operated on temporary tables
+-- 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.
+\c -
+SET search_path TO 'pg_temp';
+BEGIN;
+SELECT current_schema() ~ 'pg_temp' AS is_temp_schema;
+ is_temp_schema 
+----------------
+ t
+(1 row)
+
+PREPARE TRANSACTION 'twophase_search';
+ERROR:  cannot PREPARE a transaction that has operated on temporary namespace
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index 1beccc6ceb..e634ddb9ca 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -224,3 +224,59 @@ select * from temp_inh_oncommit_test;
 -- one relation remains
 select relname from pg_class where relname like 'temp_inh_oncommit_test%';
 drop table temp_inh_oncommit_test;
+
+-- Tests with two-phase commit
+-- Transactions creating objects in a temporary namespace cannot be used
+-- with two-phase commit.
+
+-- These cases generate errors about temporary namespace.
+-- Function creation
+begin;
+create function pg_temp.twophase_func() returns void as
+  $$ select '2pc_func'::text $$ language sql;
+prepare transaction 'twophase_func';
+-- 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';
+-- Operator creation
+begin;
+create operator pg_temp.@@ (leftarg = int4, rightarg = int4, procedure = int4mi);
+prepare transaction 'twophase_operator';
+
+-- These generate errors about temporary tables.
+begin;
+create type pg_temp.twophase_type as (a int);
+prepare transaction 'twophase_type';
+begin;
+create view pg_temp.twophase_view as select 1;
+prepare transaction 'twophase_view';
+begin;
+create sequence pg_temp.twophase_seq;
+prepare transaction 'twophase_sequence';
+
+-- Temporary tables cannot be used with two-phase commit.
+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;
+lock twophase_tab in access exclusive mode;
+prepare transaction 'twophase_tab';
+begin;
+drop table twophase_tab;
+prepare transaction 'twophase_tab';
+
+-- 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.
+\c -
+SET search_path TO 'pg_temp';
+BEGIN;
+SELECT current_schema() ~ 'pg_temp' AS is_temp_schema;
+PREPARE TRANSACTION 'twophase_search';
-- 
2.20.1

