v56-0001-refactor-v56-check_partitions_for_split.no-cfbot

application/octet-stream

Filename: v56-0001-refactor-v56-check_partitions_for_split.no-cfbot
Type: application/octet-stream
Part: 0
Message: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
From 2c9cf178e36befbad30ce9030b899bb27f79ad4d Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Tue, 16 Sep 2025 16:42:17 +0800
Subject: [PATCH v56 1/1] refactor v56 check_partitions_for_split

move some of error handling in check_partitions_for_split to
transformPartitionCmdForSplit.
---
 src/backend/parser/parse_utilcmd.c            | 112 ++++++++++++-
 src/backend/partitioning/partbounds.c         | 157 +++++-------------
 src/test/regress/expected/partition_split.out |  15 +-
 3 files changed, 161 insertions(+), 123 deletions(-)

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index e2efe0bf0c9..1b7a270ca51 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3565,10 +3565,24 @@ static void
 transformPartitionCmdForSplit(CreateStmtContext *cxt, PartitionCmd *partcmd)
 {
 	Relation	parent = cxt->rel;
+	PartitionKey key;
+	char		strategy;
 	Oid			splitPartOid;
+	Oid			defaultPartOid;
+	int			i = 0;
+	int			default_index = -1;
+	bool		isSplitPartDefault;
+	ListCell   *listptr,
+			   *listptr2;
+	List		*splitlist;
+
+	splitlist = partcmd->partlist;
+	key = RelationGetPartitionKey(parent);
+	strategy = get_partition_strategy(key);
+	defaultPartOid = get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true));
 
 	/* Transform partition bounds for all partitions in the list: */
-	foreach_node(SinglePartitionSpec, sps, partcmd->partlist)
+	foreach_node(SinglePartitionSpec, sps, splitlist)
 	{
 		cxt->partbound = NULL;
 		transformPartitionCmd(cxt, sps->bound);
@@ -3587,8 +3601,102 @@ transformPartitionCmdForSplit(CreateStmtContext *cxt, PartitionCmd *partcmd)
 
 	checkPartition(parent, splitPartOid, false);
 
+	switch (strategy)
+	{
+		case PARTITION_STRATEGY_LIST:
+		case PARTITION_STRATEGY_RANGE:
+			{
+				foreach_node(SinglePartitionSpec, sps, splitlist)
+				{
+					if (sps->bound->is_default)
+					{
+						default_index = foreach_current_index(sps);
+						i++;
+					}
+
+					if (i > 1)
+						ereport(ERROR,
+								errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+								errmsg("DEFAULT partition should be one"),
+								parser_errposition(cxt->pstate, sps->name->location));
+				}
+			}
+			break;
+
+		case PARTITION_STRATEGY_HASH:
+			ereport(ERROR,
+					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("partition of hash-partitioned table cannot be split"));
+			break;
+
+		default:
+			elog(ERROR, "unexpected partition strategy: %d",
+				 (int) key->strategy);
+			break;
+	}
+
+	/* isSplitPartDefault: is the being split partition a DEFAULT partition? */
+	isSplitPartDefault = (defaultPartOid == splitPartOid);
+
+	if (isSplitPartDefault && default_index == -1)
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				errmsg("can not split DEFAULT partition \"%s\"",
+					   get_rel_name(splitPartOid)),
+				errhint("To split DEFAULT partition one of the new partition msut be DEFAULT"),
+				parser_errposition(cxt->pstate, ((SinglePartitionSpec *) linitial(splitlist))->name->location));
+
+	/*
+	 * If the partition being split is not DEFAULT and DEFAULT partition exists,
+	 * then the resulting split partitions cannot be DEFAULT.
+	*/
+	if (!isSplitPartDefault && (default_index != -1) && OidIsValid(defaultPartOid))
+	{
+		SinglePartitionSpec *spsDef =
+			(SinglePartitionSpec *) list_nth(splitlist, default_index);
+
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				errmsg("can not split non-DEFAULT partition \"%s\"",
+					   get_rel_name(splitPartOid)),
+				errmsg("New partition cannot be DEFAULT because DEFAULT partition \"%s\" already exists",
+					   get_rel_name(defaultPartOid)),
+				parser_errposition(cxt->pstate, spsDef->name->location));
+	}
+
+	foreach(listptr, splitlist)
+	{
+		Oid			nspid;
+		SinglePartitionSpec   *sps = (SinglePartitionSpec *) lfirst(listptr);
+		RangeVar   *name = sps->name;
+
+		nspid = RangeVarGetCreationNamespace(sps->name);
+
+		/* Partitions in the list should have different names. */
+		for_each_cell(listptr2, splitlist, lnext(splitlist, listptr))
+		{
+			Oid		nspid2;
+			SinglePartitionSpec   *sps2 = (SinglePartitionSpec *) lfirst(listptr2);
+			RangeVar   *name2 = sps2->name;
+
+			if (equal(name, name2))
+				ereport(ERROR,
+						errcode(ERRCODE_DUPLICATE_TABLE),
+						errmsg("partition with name \"%s\" is already used", name->relname),
+						parser_errposition(cxt->pstate, name2->location));
+
+			nspid2 = RangeVarGetCreationNamespace(sps2->name);
+
+			if (nspid2 == nspid && strcmp(name->relname, name2->relname) == 0)
+				ereport(ERROR,
+						errcode(ERRCODE_DUPLICATE_TABLE),
+						errmsg("partition with name \"%s\" is already used", name->relname),
+						parser_errposition(cxt->pstate, name2->location));
+		}
+	}
+
 	/* Then we should check partitions with transformed bounds. */
-	check_partitions_for_split(parent, splitPartOid, partcmd->partlist, cxt->pstate);
+	check_partitions_for_split(parent, splitPartOid, splitlist, cxt->pstate);
 }
 
 
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 0a04d628b6a..168d92bdf8c 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5446,7 +5446,7 @@ check_partition_bounds_for_split_list(Relation parent, char *relname,
 	 */
 	foreach_node(Const, val, spec->listdatums)
 	{
-		overlap_location = val->location;
+		overlap_location = exprLocation((Node *) val);
 		if (!val->constisnull)
 		{
 			int			offset;
@@ -5469,8 +5469,9 @@ check_partition_bounds_for_split_list(Relation parent, char *relname,
 			else
 				ereport(ERROR,
 						errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-						errmsg("new partition \"%s\" cannot have this value because split partition does not have",
-							   relname),
+						errmsg("new partition \"%s\" cannot have this value because split partition \"%s\" does not have",
+							   relname,
+							   get_rel_name(splitPartOid)),
 						parser_errposition(pstate, overlap_location));
 		}
 		else if (partition_bound_accepts_nulls(boundinfo))
@@ -5485,8 +5486,9 @@ check_partition_bounds_for_split_list(Relation parent, char *relname,
 		else
 			ereport(ERROR,
 					errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-					errmsg("new partition \"%s\" cannot have NULL value because split partition does not have",
-						   relname),
+					errmsg("new partition \"%s\" cannot have NULL value because split partition \"%s\" does not have",
+						   relname,
+						   get_rel_name(splitPartOid)),
 					parser_errposition(pstate, overlap_location));
 	}
 
@@ -5639,19 +5641,12 @@ check_parent_values_in_new_partitions(Relation parent,
  * check_partitions_for_split
  *
  * Checks new partitions for SPLIT PARTITIONS command:
- * 1. DEFAULT partition should be at most one.
- * 2. New partitions should have different names
- *    (with existing partitions too).
- * 3. Bounds of new partitions should not overlap with new and existing
+ * 1. Bounds of new partitions should not overlap with new and existing
  *    partitions.
- * 4. In case split partition is DEFAULT partition, one of new partitions
- *    should be DEFAULT.
- * 5. In case new partitions or existing partitions contains DEFAULT
+ * 2. In case new partitions or existing partitions contains DEFAULT
  *    partition, new partitions can have any bounds inside split
  *    partition bound (can be spaces between partitions bounds).
- * 6. In case partitioned table does not have DEFAULT partition, DEFAULT
- *    partition can be defined as one of new partition.
- * 7. In case new partitions not contains DEFAULT partition and
+ * 3. In case new partitions not contains DEFAULT partition and
  *    partitioned table does not have DEFAULT partition the following
  *    should be true: sum bounds of new partitions should be equal
  *    to bound of split partition.
@@ -5671,59 +5666,49 @@ check_partitions_for_split(Relation parent,
 	char		strategy;
 	Oid			defaultPartOid;
 	bool		isSplitPartDefault;
-	bool		createDefaultPart;
+	bool		createDefaultPart = false;
 	int			default_index = -1;
-	int			i,
-				j;
+	int			i;
 	SinglePartitionSpec **new_parts;
 	SinglePartitionSpec *spsPrev = NULL;
+
+	/*
+	 * nparts count number of split partitions, but it exclude the default
+	 * partition
+	 */
 	int			nparts = 0;
 
 	key = RelationGetPartitionKey(parent);
 	strategy = get_partition_strategy(key);
 
-	switch (strategy)
-	{
-		case PARTITION_STRATEGY_LIST:
-		case PARTITION_STRATEGY_RANGE:
-			{
-				/*
-				 * Make array new_parts with new partitions except DEFAULT
-				 * partition.
-				 */
-				new_parts = (SinglePartitionSpec **)
-					palloc0(list_length(partlist) * sizeof(SinglePartitionSpec *));
-				i = 0;
-				foreach_node(SinglePartitionSpec, sps, partlist)
-				{
-					if (sps->bound->is_default)
-					{
-						if (default_index >= 0)
-							ereport(ERROR,
-									errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-									errmsg("DEFAULT partition should be one"),
-									parser_errposition(pstate, sps->name->location));
-						default_index = i;
-					}
-					else
-					{
-						new_parts[nparts++] = sps;
-					}
-					i++;
-				}
-			}
-			break;
+	defaultPartOid =
+		get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true));
+
+	Assert(strategy == PARTITION_STRATEGY_RANGE ||
+		   strategy == PARTITION_STRATEGY_LIST);
+
+	/*
+	 * Make array new_parts with new partitions except DEFAULT partition.
+	*/
+	new_parts = (SinglePartitionSpec **)
+		palloc0(list_length(partlist) * sizeof(SinglePartitionSpec *));
 
-		case PARTITION_STRATEGY_HASH:
-			ereport(ERROR,
-					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					errmsg("partition of hash-partitioned table cannot be split"));
-			break;
+	/* isSplitPartDefault flag: is split partition a DEFAULT partition? */
+	isSplitPartDefault = (defaultPartOid == splitPartOid);
 
-		default:
-			elog(ERROR, "unexpected partition strategy: %d",
-				 (int) key->strategy);
-			break;
+	foreach_node(SinglePartitionSpec, sps, partlist)
+	{
+		if (sps->bound->is_default)
+			default_index = foreach_current_index(sps);
+		else
+			new_parts[nparts++] = sps;
+	}
+
+	/* Indicator that the DEFAULT partition will be created. */
+	if (default_index != -1)
+	{
+		createDefaultPart = true;
+		Assert(nparts == list_length(partlist) - 1);
 	}
 
 	if (strategy == PARTITION_STRATEGY_RANGE)
@@ -5761,33 +5746,6 @@ check_partitions_for_split(Relation parent,
 		pfree(lower_bounds);
 	}
 
-	defaultPartOid =
-		get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true));
-
-	/* isSplitPartDefault flag: is split partition a DEFAULT partition? */
-	isSplitPartDefault = (defaultPartOid == splitPartOid);
-
-	if (isSplitPartDefault && default_index < 0)
-	{
-		ereport(ERROR,
-				errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				errmsg("one partition in the list should be DEFAULT because split partition is DEFAULT"),
-				parser_errposition(pstate, ((SinglePartitionSpec *) linitial(partlist))->name->location));
-	}
-	else if (!isSplitPartDefault && (default_index >= 0) && OidIsValid(defaultPartOid))
-	{
-		SinglePartitionSpec *spsDef =
-			(SinglePartitionSpec *) list_nth(partlist, default_index);
-
-		ereport(ERROR,
-				errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				errmsg("new partition cannot be DEFAULT because DEFAULT partition already exists"),
-				parser_errposition(pstate, spsDef->name->location));
-	}
-
-	/* Indicator that the DEFAULT partition will be created. */
-	createDefaultPart = (default_index >= 0);
-
 	for (i = 0; i < nparts; i++)
 	{
 		SinglePartitionSpec *sps = new_parts[i];
@@ -5833,35 +5791,6 @@ check_partitions_for_split(Relation parent,
 											  pstate);
 
 		spsPrev = sps;
-
-		/* Check: new partitions should have different names. */
-		for (j = i + 1; j < nparts; j++)
-		{
-			SinglePartitionSpec *sps2 = new_parts[j];
-			bool		result = false;
-
-			/*
-			 * Need to compare namespaces? One of the schema names may be
-			 * undefined, but the schemas may still be equal.
-			 */
-			if ((sps->name->schemaname && !sps2->name->schemaname) ||
-				(!sps->name->schemaname && sps2->name->schemaname))
-			{
-				Oid			nspid = RangeVarGetCreationNamespace(sps->name);
-				Oid			nspid2 = RangeVarGetCreationNamespace(sps2->name);
-
-				if (nspid == nspid2)
-					result = (strcmp(sps->name->relname, sps2->name->relname) == 0);
-			}
-			else
-				result = equal(sps->name, sps2->name);
-
-			if (result)
-				ereport(ERROR,
-						errcode(ERRCODE_DUPLICATE_TABLE),
-						errmsg("name \"%s\" is already used", sps2->name->relname),
-						parser_errposition(pstate, sps2->name->location));
-		}
 	}
 
 	if (strategy == PARTITION_STRATEGY_LIST)
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 886e5e0ab7e..e328838474f 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -62,7 +62,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
   (PARTITION sales_feb_mar_apr2022 FOR VALUES FROM ('2022-02-01') TO ('2022-03-01'),
    PARTITION sales_feb_mar_apr2022 FOR VALUES FROM ('2022-03-01') TO ('2022-04-01'),
    PARTITION sales_apr2022 FOR VALUES FROM ('2022-04-01') TO ('2022-05-01'));
-ERROR:  name "sales_feb_mar_apr2022" is already used
+ERROR:  partition with name "sales_feb_mar_apr2022" is already used
 LINE 3:    PARTITION sales_feb_mar_apr2022 FOR VALUES FROM ('2022-03...
                      ^
 -- ERROR:  name "sales_feb2022" is already used
@@ -70,7 +70,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
   (PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO ('2022-03-01'),
    PARTITION sales_feb2022 FOR VALUES FROM ('2022-03-01') TO ('2022-04-01'),
    PARTITION sales_apr2022 FOR VALUES FROM ('2022-04-01') TO ('2022-05-01'));
-ERROR:  name "sales_feb2022" is already used
+ERROR:  partition with name "sales_feb2022" is already used
 LINE 3:    PARTITION sales_feb2022 FOR VALUES FROM ('2022-03-01') TO...
                      ^
 -- ERROR:  name "sales_feb2022" is already used
@@ -78,7 +78,7 @@ ALTER TABLE sales_range SPLIT PARTITION sales_feb_mar_apr2022 INTO
   (PARTITION sales_feb2022 FOR VALUES FROM ('2022-02-01') TO ('2022-03-01'),
    PARTITION partition_split_schema.sales_feb2022 FOR VALUES FROM ('2022-03-01') TO ('2022-04-01'),
    PARTITION sales_apr2022 FOR VALUES FROM ('2022-04-01') TO ('2022-05-01'));
-ERROR:  name "sales_feb2022" is already used
+ERROR:  partition with name "sales_feb2022" is already used
 LINE 3:    PARTITION partition_split_schema.sales_feb2022 FOR VALUES...
                      ^
 -- ERROR:  ALTER action SPLIT PARTITION cannot be performed on relation "sales_feb_mar_apr2022"
@@ -509,9 +509,10 @@ ALTER TABLE sales_range SPLIT PARTITION sales_others INTO
   (PARTITION sales_dec2021 FOR VALUES FROM (20211201) TO (20220101),
    PARTITION sales_jan2022 FOR VALUES FROM (20220101) TO (20220201),
    PARTITION sales_feb2022 FOR VALUES FROM (20220201) TO (20220301));
-ERROR:  one partition in the list should be DEFAULT because split partition is DEFAULT
+ERROR:  can not split DEFAULT partition "sales_others"
 LINE 2:   (PARTITION sales_dec2021 FOR VALUES FROM (20211201) TO (20...
                      ^
+HINT:  To split DEFAULT partition one of the new partition msut be DEFAULT
 -- no error: bounds of sales_noerror are between sales_dec2021 and sales_feb2022
 ALTER TABLE sales_range SPLIT PARTITION sales_others INTO
   (PARTITION sales_dec2021 FOR VALUES FROM (20211201) TO (20220101),
@@ -915,7 +916,7 @@ ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
   (PARTITION sales_west FOR VALUES IN ('Lisbon', 'New York', 'Madrid', NULL),
    PARTITION sales_east FOR VALUES IN ('Bejing', 'Delhi', 'Vladivostok'),
    PARTITION sales_central FOR VALUES IN ('Warsaw', 'Berlin', 'Kyiv'));
-ERROR:  new partition "sales_west" cannot have NULL value because split partition does not have
+ERROR:  new partition "sales_west" cannot have NULL value because split partition "sales_all" does not have
 LINE 2: ...s_west FOR VALUES IN ('Lisbon', 'New York', 'Madrid', NULL),
                                                                  ^
 -- ERROR:  new partition "sales_west" cannot have this value because split partition does not have
@@ -923,7 +924,7 @@ ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
   (PARTITION sales_west FOR VALUES IN ('Lisbon', 'New York', 'Madrid', 'Melbourne'),
    PARTITION sales_east FOR VALUES IN ('Bejing', 'Delhi', 'Vladivostok'),
    PARTITION sales_central FOR VALUES IN ('Warsaw', 'Berlin', 'Kyiv'));
-ERROR:  new partition "sales_west" cannot have this value because split partition does not have
+ERROR:  new partition "sales_west" cannot have this value because split partition "sales_all" does not have
 LINE 2: ...st FOR VALUES IN ('Lisbon', 'New York', 'Madrid', 'Melbourne...
                                                              ^
 -- ERROR:  new partition cannot be DEFAULT because DEFAULT partition already exists
@@ -932,7 +933,7 @@ ALTER TABLE sales_list SPLIT PARTITION sales_all INTO
    PARTITION sales_east FOR VALUES IN ('Bejing', 'Delhi', 'Vladivostok'),
    PARTITION sales_central FOR VALUES IN ('Warsaw', 'Berlin', 'Kyiv'),
    PARTITION sales_others2 DEFAULT);
-ERROR:  new partition cannot be DEFAULT because DEFAULT partition already exists
+ERROR:  New partition cannot be DEFAULT because DEFAULT partition "sales_others" already exists
 LINE 5:    PARTITION sales_others2 DEFAULT);
                      ^
 DROP TABLE sales_list;
-- 
2.34.1