v7-0002-Verify-persistence-of-new-partitions-during-MERGE.patch

From ca4d010f40b1c10cfb72d72e41fda3c992f66235 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 18 Apr 2024 12:41:09 +0300
Subject: [PATCH v7 2/7] Verify persistence of new partitions during
 MERGE/SPLIT operations

The createPartitionTable() function is responsible for creating new partitions
for ALTER TABLE ... MERGE PARTITIONS, and ALTER TABLE ... SPLIT PARTITION
commands.  It emulates the behaviour of CREATE TABLE ... (LIKE ...), but
lacks of check for the result relation persistence.  This commit adds
corresponding check similarly to what we have for CREATE TABLE ...
PARTITION OF command.

This commit also changes the signature of createPartitionTable() making it
take the parent's Relation itself instead of the name of the parent relation,
and return the Relation of new partition.  That doesn't lead to
complications, because both callers have the parent table open and need to
open the new partition.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
Author: Dmitry Koval
Reviewed-by: Alexander Korotkov, Robert Haas
---
 src/backend/commands/tablecmds.c              | 70 +++++++++------
 src/test/regress/expected/partition_merge.out | 89 +++++++++++++++++++
 src/test/regress/expected/partition_split.out | 29 ++++++
 src/test/regress/sql/partition_merge.sql      | 68 ++++++++++++++
 src/test/regress/sql/partition_split.sql      | 23 +++++
 5 files changed, 254 insertions(+), 25 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0f72ffa9abc..2025e68bdae 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21182,18 +21182,27 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
 
 /*
  * createPartitionTable: create table for a new partition with given name
- * (newPartName) like table (modelRelName)
+ * (newPartName) like table (modelRel)
  *
- * Emulates command: CREATE TABLE <newPartName> (LIKE <modelRelName>
+ * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
  * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
+ * Function returns the created relation (locked in AccessExclusiveLock mode).
  */
-static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+static Relation
+createPartitionTable(RangeVar *newPartName, Relation modelRel,
 					 AlterTableUtilityContext *context)
 {
 	CreateStmt *createStmt;
 	TableLikeClause *tlc;
 	PlannedStmt *wrapper;
+	Relation	newRel;
+
+	/* If existing rel is temp, it must belong to this session */
+	if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
+		!modelRel->rd_islocaltemp)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("cannot create as partition of temporary relation of another session")));
 
 	createStmt = makeNode(CreateStmt);
 	createStmt->relation = newPartName;
@@ -21206,7 +21215,8 @@ createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
 	createStmt->if_not_exists = false;
 
 	tlc = makeNode(TableLikeClause);
-	tlc->relation = modelRelName;
+	tlc->relation = makeRangeVar(get_namespace_name(RelationGetNamespace(modelRel)),
+								 RelationGetRelationName(modelRel), -1);
 
 	/*
 	 * Indexes will be inherited on "attach new partitions" stage, after data
@@ -21232,6 +21242,33 @@ createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
 				   NULL,
 				   None_Receiver,
 				   NULL);
+
+	/*
+	 * Open the new partition with no lock, because we already have
+	 * AccessExclusiveLock placed there after creation.
+	 */
+	newRel = table_openrv(newPartName, NoLock);
+
+	/*
+	 * If the parent is permanent, so must be all of its partitions.  Note
+	 * that inheritance allows that case.
+	 */
+	if (modelRel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+		newRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("cannot create a temporary relation as partition of permanent relation \"%s\"",
+						RelationGetRelationName(modelRel))));
+
+	/* Permanent rels cannot inherit from temporary ones */
+	if (newRel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+		modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("cannot create a permanent relation as partition of temporary relation \"%s\"",
+						RelationGetRelationName(modelRel))));
+
+	return newRel;
 }
 
 /*
@@ -21251,7 +21288,6 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	char		tmpRelName[NAMEDATALEN];
 	List	   *newPartRels = NIL;
 	ObjectAddress object;
-	RangeVar   *parentName;
 	Oid			defaultPartOid;
 
 	defaultPartOid = get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true));
@@ -21323,18 +21359,12 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	}
 
 	/* Create new partitions (like split partition), without indexes. */
-	parentName = makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-							  RelationGetRelationName(rel), -1);
 	foreach(listptr, cmd->partlist)
 	{
 		SinglePartitionSpec *sps = (SinglePartitionSpec *) lfirst(listptr);
 		Relation	newPartRel;
 
-		createPartitionTable(sps->name, parentName, context);
-
-		/* Open the new partition and acquire exclusive lock on it. */
-		newPartRel = table_openrv(sps->name, AccessExclusiveLock);
-
+		newPartRel = createPartitionTable(sps->name, rel, context);
 		newPartRels = lappend(newPartRels, newPartRel);
 	}
 
@@ -21538,18 +21568,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		DetachPartitionFinalize(rel, mergingPartition, false, defaultPartOid);
 	}
 
-	createPartitionTable(cmd->name,
-						 makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-									  RelationGetRelationName(rel), -1),
-						 context);
-
-	/*
-	 * Open the new partition and acquire exclusive lock on it.  This will
-	 * stop all the operations with partitioned table.  This might seem
-	 * excessive, but this is the way we make sure nobody is planning queries
-	 * involving merging partitions.
-	 */
-	newPartRel = table_openrv(cmd->name, AccessExclusiveLock);
+	/* Create table for new partition, use partitioned table as model. */
+	newPartRel = createPartitionTable(cmd->name, rel, context);
 
 	/* Copy data from merged partitions to new partition. */
 	moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 2e0bfdc705d..cf961b041fb 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -746,6 +746,36 @@ DROP TABLE t3;
 DROP TABLE t2;
 DROP TABLE t1;
 --
+-- Try to MERGE partitions of temporary table.
+--
+CREATE TEMP TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TEMP TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TEMP TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | t
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | t
+(2 rows)
+
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  cannot create a permanent relation as partition of temporary relation "t"
+-- Partition should be temporary.
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | t
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | t
+(2 rows)
+
+DROP TABLE t;
+--
 -- Check the partition index name if the partition name is the same as one
 -- of the merged partitions.
 --
@@ -771,4 +801,63 @@ Not-null constraints:
 
 DROP TABLE t;
 --
+-- Try mixing permanent and temporary partitions.
+--
+SET search_path = public, pg_temp;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c WHERE c.oid = 't'::regclass;
+ oid | relpersistence 
+-----+----------------
+ t   | p
+(1 row)
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | p
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | p
+(2 rows)
+
+SET search_path = pg_temp, public;
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  cannot create a temporary relation as partition of permanent relation "t"
+RESET search_path;
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO pg_temp.tp_0_2;
+ERROR:  cannot create a temporary relation as partition of permanent relation "t"
+DROP TABLE t;
+SET search_path = pg_temp, public;
+BEGIN;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c WHERE c.oid = 't'::regclass;
+ oid | relpersistence 
+-----+----------------
+ t   | t
+(1 row)
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | t
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | t
+(2 rows)
+
+SET search_path = public, pg_temp;
+-- Can't merge temporary partitions into a persistent partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ERROR:  cannot create a permanent relation as partition of temporary relation "t"
+ROLLBACK;
+RESET search_path;
+--
 DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 419d169f036..660361cd20e 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1427,4 +1427,33 @@ ERROR:  relation "t1pa" is not a partition of relation "t2"
 DROP TABLE t2;
 DROP TABLE t1;
 --
+-- Try to SPLIT partition of temporary table.
+--
+CREATE TEMP TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TEMP TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_2 | FOR VALUES FROM (0) TO (2) | t
+(1 row)
+
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+ERROR:  cannot create a permanent relation as partition of temporary relation "t"
+-- Partitions should be temporary.
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+  oid   |        pg_get_expr         | relpersistence 
+--------+----------------------------+----------------
+ tp_0_2 | FOR VALUES FROM (0) TO (2) | t
+(1 row)
+
+DROP TABLE t;
+--
 DROP SCHEMA partition_split_schema;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 72b1cb0b35e..29ae6a09f13 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -444,6 +444,28 @@ DROP TABLE t3;
 DROP TABLE t2;
 DROP TABLE t1;
 
+--
+-- Try to MERGE partitions of temporary table.
+--
+CREATE TEMP TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TEMP TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TEMP TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+
+-- Partition should be temporary.
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+DROP TABLE t;
+
 --
 -- Check the partition index name if the partition name is the same as one
 -- of the merged partitions.
@@ -462,5 +484,51 @@ ALTER TABLE t MERGE PARTITIONS (tp_1_2, tp_0_1) INTO tp_1_2;
 
 DROP TABLE t;
 
+--
+-- Try mixing permanent and temporary partitions.
+--
+SET search_path = partitions_merge_schema, pg_temp, public;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c WHERE c.oid = 't'::regclass;
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+SET search_path = pg_temp, partitions_merge_schema, public;
+
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+
+SET search_path = partitions_merge_schema, public;
+
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO pg_temp.tp_0_2;
+DROP TABLE t;
+
+SET search_path = pg_temp, partitions_merge_schema, public;
+
+BEGIN;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c WHERE c.oid = 't'::regclass;
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+SET search_path = partitions_merge_schema, pg_temp, public;
+
+-- Can't merge temporary partitions into a persistent partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ROLLBACK;
+
+RESET search_path;
+
 --
 DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index b63532ee562..576f9f0f63c 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -844,5 +844,28 @@ ALTER TABLE t2 SPLIT PARTITION t1pa INTO
 DROP TABLE t2;
 DROP TABLE t1;
 
+--
+-- Try to SPLIT partition of temporary table.
+--
+CREATE TEMP TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TEMP TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+
+-- Partitions should be temporary.
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid), c.relpersistence
+  FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+  WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+  ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT', c.oid::pg_catalog.regclass::pg_catalog.text;
+
+DROP TABLE t;
+
 --
 DROP SCHEMA partition_split_schema;
-- 
2.39.3 (Apple Git-145)