v47-0001-rename-function-argument-and-minor-refactor.no-cfbot
application/octet-stream
Filename: v47-0001-rename-function-argument-and-minor-refactor.no-cfbot
Type: application/octet-stream
Part: 0
From 1b25410e75a2029c3aec7f455bcb48e6cdac241e Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 18 Jun 2025 10:40:39 +0800
Subject: [PATCH v47 1/1] rename function argument and minor refactor
* rename "rel" or "modelRel" to "parent_rel" to improve code readability
* comments refactoring
* minor tests changes.
---
src/backend/commands/tablecmds.c | 98 +++++++++----------
src/test/regress/expected/partition_merge.out | 64 +++++-------
src/test/regress/sql/partition_merge.sql | 8 +-
3 files changed, 75 insertions(+), 95 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b724e2d7150..d7d0e349b19 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -22158,17 +22158,16 @@ evaluateGeneratedExpressionsAndCheckConstraints(AlteredTableInfo *tab,
}
/*
- * getAttributesList: return list of columns (ColumnDef) like model table
- * (modelRel)
+ * getAttributesList: build a list of columns (ColumnDef) based on parent_rel
*/
static List *
-getAttributesList(Relation modelRel)
+getAttributesList(Relation parent_rel)
{
AttrNumber parent_attno;
TupleDesc modelDesc;
List *colList = NIL;
- modelDesc = RelationGetDescr(modelRel);
+ modelDesc = RelationGetDescr(parent_rel);
for (parent_attno = 1; parent_attno <= modelDesc->natts;
parent_attno++)
@@ -22214,13 +22213,14 @@ getAttributesList(Relation modelRel)
/*
- * createTableConstraints: create constraints, default values and generated
- * values (prototype is function expandTableLikeClause).
- * tab is pending-work queue for newRel, we may need it in moveMergedTablesRows.
+ * createTableConstraints:
+ * create check constraints, default values and generated values for newRel
+ * based on parent_rel. tab is pending-work queue for newRel, we may need it in
+ * moveMergedTablesRows.
*/
static void
createTableConstraints(List **wqueue, AlteredTableInfo *tab,
- Relation modelRel, Relation newRel)
+ Relation parent_rel, Relation newRel)
{
TupleDesc tupleDesc;
TupleConstr *constr;
@@ -22230,7 +22230,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
List *Constraints = NIL;
List *cookedConstraints = NIL;
- tupleDesc = RelationGetDescr(modelRel);
+ tupleDesc = RelationGetDescr(parent_rel);
constr = tupleDesc->constr;
if (!constr)
@@ -22265,13 +22265,13 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
NewColumnValue *newval;
if (attribute->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
- this_default = build_generation_expression(modelRel, attribute->attnum);
+ this_default = build_generation_expression(parent_rel, attribute->attnum);
else
{
this_default = TupleDescGetDefault(tupleDesc, attribute->attnum);
if (this_default == NULL)
elog(ERROR, "default expression not found for attribute %d of relation \"%s\"",
- attribute->attnum, RelationGetRelationName(modelRel));
+ attribute->attnum, RelationGetRelationName(parent_rel));
}
num = attmap->attnums[parent_attno - 1];
@@ -22288,14 +22288,14 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
errmsg("cannot convert whole-row table reference"),
errdetail("Generation expression for column \"%s\" contains a whole-row reference to table \"%s\".",
NameStr(attribute->attname),
- RelationGetRelationName(modelRel)));
+ RelationGetRelationName(parent_rel)));
/* Add a pre-cooked default expression. */
StoreAttrDefault(newRel, num, def, true);
/*
- * Stored generated column expressions in modelRel might reference
- * tableoid. newRel, modelRel tableoid clear is not the same. If
+ * Stored generated column expressions in parent_rel might reference
+ * tableoid. newRel, parent_rel tableoid clear is not the same. If
* so, these stored generated columns require recomputation for
* newRel within moveMergedTablesRows.
*/
@@ -22345,7 +22345,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
errmsg("cannot convert whole-row table reference"),
errdetail("Constraint \"%s\" contains a whole-row reference to table \"%s\".",
ccname,
- RelationGetRelationName(modelRel)));
+ RelationGetRelationName(parent_rel)));
constr = makeNode(Constraint);
constr->contype = CONSTR_CHECK;
@@ -22370,7 +22370,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
CommandCounterIncrement();
/*
- * modelRel check constraint expresssion may reference tableoid, so later in
+ * parent_rel check constraint expresssion may reference tableoid, so later in
* moveMergedTablesRows, we need evulate the check constraint again for the
* newRel. We can check weather check constraint contain tableoid reference
* or not via pull_varattnos.
@@ -22386,8 +22386,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
pull_varattnos(qual, 1, &attnums);
/*
- * Add check only if it contains tableoid
- * (TableOidAttributeNumber).
+ * Add check only if it contains tableoid (TableOidAttributeNumber).
*/
if (bms_is_member(TableOidAttributeNumber - FirstLowInvalidHeapAttributeNumber,
attnums))
@@ -22416,7 +22415,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
* The "include_noinh" argument is false because a partitioned table
* cannot have NO INHERIT constraint.
*/
- nnconstraints = RelationGetNotNullConstraints(RelationGetRelid(modelRel),
+ nnconstraints = RelationGetNotNullConstraints(RelationGetRelid(parent_rel),
false, false);
Assert(list_length(nnconstraints) > 0);
@@ -22431,20 +22430,18 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
/*
- * createPartitionTable: create table for a new partition with given name
- * (newPartName) like table (modelRel, partitioned table). ownerId is
- * determined by the partition on which the operation is performed, so it
- * is passed separately.
+ * createPartitionTable:
*
- * Also, this function sets the new partition access method same as parent
- * table access methods (similarly to CREATE TABLE ... PARTITION OF). It
- * checks that parent and child tables have compatible persistence.
+ * Create a new partition (newPartName) for partitioned table (parent_rel).
+ * ownerId is determined by the partition on which the operation is performed,
+ * so it is passed separately. The new partition will inherit the access method
+ * and persistence type from the parent table.
*
- * Function returns the created relation (locked in AccessExclusiveLock mode).
+ * returns the created relation (locked in AccessExclusiveLock mode).
*/
static Relation
createPartitionTable(List **wqueue, RangeVar *newPartName,
- Relation modelRel, Oid ownerId)
+ Relation parent_rel, Oid ownerId)
{
Relation newRel;
Oid newRelId;
@@ -22456,19 +22453,19 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
AlteredTableInfo *new_partrel_tab;
/* If existing rel is temp, it must belong to this session */
- if (RELATION_IS_OTHER_TEMP(modelRel))
+ if (RELATION_IS_OTHER_TEMP(parent_rel))
ereport(ERROR,
errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot create as partition of temporary relation of another session"));
/* Look up inheritance ancestors and generate relation schema. */
- colList = getAttributesList(modelRel);
+ colList = getAttributesList(parent_rel);
/* Create a tuple descriptor from the relation schema. */
descriptor = BuildDescForRelation(colList);
/* Look up the access method for new relation. */
- relamId = (modelRel->rd_rel->relam != InvalidOid) ? modelRel->rd_rel->relam : HEAP_TABLE_AM_OID;
+ relamId = (parent_rel->rd_rel->relam != InvalidOid) ? parent_rel->rd_rel->relam : HEAP_TABLE_AM_OID;
/* Look up the namespace in which we are supposed to create the relation. */
namespaceId =
@@ -22481,7 +22478,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
/* Create the relation. */
newRelId = heap_create_with_catalog(newPartName->relname,
namespaceId,
- modelRel->rd_rel->reltablespace,
+ parent_rel->rd_rel->reltablespace,
InvalidOid,
InvalidOid,
InvalidOid,
@@ -22497,7 +22494,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
(Datum) 0,
true,
allowSystemTableMods,
- false,
+ true,
InvalidOid,
NULL);
@@ -22522,23 +22519,23 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
* affected by the search_path. If the parent is permanent, so must be
* all of its partitions.
*/
- if (modelRel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
+ if (parent_rel->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)));
+ RelationGetRelationName(parent_rel)));
/* Permanent rels cannot be partitions belonging to temporary parent */
if (newRel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
- modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
+ parent_rel->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)));
+ RelationGetRelationName(parent_rel)));
/* Create constraints, default values and generated values */
- createTableConstraints(wqueue, new_partrel_tab, modelRel, newRel);
+ createTableConstraints(wqueue, new_partrel_tab, parent_rel, newRel);
/*
* Need to call CommandCounterIncrement, so fresh relcache entry have newly
@@ -22831,9 +22828,19 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
defaultPartOid =
get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true));
+ /* Detach all merged partitions */
+ foreach_oid(mergingPartitionOid, mergingPartitions)
+ {
+ Relation child_rel;
+
+ child_rel = table_open(mergingPartitionOid, NoLock);
+
+ detachPartitionTable(rel, child_rel, defaultPartOid);
+
+ table_close(child_rel, NoLock);
+ }
+
/*
- * Detach all merged partitions.
- *
* Perform a preliminary check to determine whether it's safe to drop all
* merging partitions before we actually do so later. After merging rows
* into the new partitions via moveMergedTablesRows, all old partitions need
@@ -22841,17 +22848,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
* merge process (moveMergedTablesRows) can be time-consuming, performing an
* early check on the drop eligibility of old partitions is preferable.
*/
- foreach_oid(mergingPartitionOid, mergingPartitions)
- {
- Relation child_rel;
-
- child_rel = table_open(mergingPartitionOid, NoLock);
-
- detachPartitionTable(rel, child_rel, defaultPartOid);
-
- table_close(child_rel, NoLock);
- }
-
foreach_oid(mergingPartitionOid, mergingPartitions)
{
ObjectAddress object;
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index f5e6ec6184a..531f2021aad 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -48,6 +48,12 @@ LINE 1: ...e MERGE PARTITIONS (sales_feb2022, sales_mar2022, partitions...
--ERROR, sales_apr_2 already exists
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_jan2022) INTO sales_apr_2;
ERROR: relation "sales_apr_2" already exists
+CREATE VIEW jan2022v as SELECT * FROM sales_jan2022;
+ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_feb2022) INTO sales_dec_jan_feb2022;
+ERROR: cannot drop table sales_jan2022 because other objects depend on it
+DETAIL: view jan2022v depends on table sales_jan2022
+HINT: Use DROP ... CASCADE to drop the dependent objects too.
+DROP VIEW jan2022v;
-- NO ERROR: test for custom partitions order, source partitions not in the search_path
SET search_path = partitions_merge_schema2, public;
ALTER TABLE partitions_merge_schema.sales_range MERGE PARTITIONS (
@@ -587,49 +593,25 @@ EXECUTE get_partition_info('{sales_list}');
sales_others | p | r | f | DEFAULT
(3 rows)
-SELECT * FROM sales_list;
- salesperson_id | salesperson_name | sales_state | sales_amount | sales_date
-----------------+------------------+----------------+--------------+------------
- 2 | Smirnoff | New York | 500 | 03-03-2022
- 5 | Deev | Lisbon | 250 | 03-07-2022
- 11 | Muller | Madrid | 650 | 03-05-2022
- 14 | Plato | Lisbon | 950 | 03-05-2022
- 1 | Trump | Bejing | 1000 | 03-01-2022
- 8 | Li | Vladivostok | 1150 | 03-09-2022
- 4 | Ivanov | Warsaw | 750 | 03-04-2022
- 6 | Poirot | Berlin | 1000 | 03-01-2022
- 12 | Smith | Kyiv | 350 | 03-10-2022
- 13 | Gandi | Warsaw | 150 | 03-08-2022
- 3 | Ford | St. Petersburg | 2000 | 03-05-2022
- 7 | May | Helsinki | 1200 | 03-06-2022
- 9 | May | Helsinki | 1200 | 03-11-2022
- 10 | Halder | Oslo | 800 | 03-02-2022
+SELECT tableoid::regclass, * FROM sales_list ORDER BY tableoid, salesperson_id;
+ tableoid | salesperson_id | salesperson_name | sales_state | sales_amount | sales_date
+------------+----------------+------------------+----------------+--------------+------------
+ sales_nord | 3 | Ford | St. Petersburg | 2000 | 03-05-2022
+ sales_nord | 7 | May | Helsinki | 1200 | 03-06-2022
+ sales_nord | 9 | May | Helsinki | 1200 | 03-11-2022
+ sales_nord | 10 | Halder | Oslo | 800 | 03-02-2022
+ sales_all | 1 | Trump | Bejing | 1000 | 03-01-2022
+ sales_all | 2 | Smirnoff | New York | 500 | 03-03-2022
+ sales_all | 4 | Ivanov | Warsaw | 750 | 03-04-2022
+ sales_all | 5 | Deev | Lisbon | 250 | 03-07-2022
+ sales_all | 6 | Poirot | Berlin | 1000 | 03-01-2022
+ sales_all | 8 | Li | Vladivostok | 1150 | 03-09-2022
+ sales_all | 11 | Muller | Madrid | 650 | 03-05-2022
+ sales_all | 12 | Smith | Kyiv | 350 | 03-10-2022
+ sales_all | 13 | Gandi | Warsaw | 150 | 03-08-2022
+ sales_all | 14 | Plato | Lisbon | 950 | 03-05-2022
(14 rows)
-SELECT * FROM sales_nord;
- salesperson_id | salesperson_name | sales_state | sales_amount | sales_date
-----------------+------------------+----------------+--------------+------------
- 3 | Ford | St. Petersburg | 2000 | 03-05-2022
- 7 | May | Helsinki | 1200 | 03-06-2022
- 9 | May | Helsinki | 1200 | 03-11-2022
- 10 | Halder | Oslo | 800 | 03-02-2022
-(4 rows)
-
-SELECT * FROM sales_all;
- salesperson_id | salesperson_name | sales_state | sales_amount | sales_date
-----------------+------------------+-------------+--------------+------------
- 2 | Smirnoff | New York | 500 | 03-03-2022
- 5 | Deev | Lisbon | 250 | 03-07-2022
- 11 | Muller | Madrid | 650 | 03-05-2022
- 14 | Plato | Lisbon | 950 | 03-05-2022
- 1 | Trump | Bejing | 1000 | 03-01-2022
- 8 | Li | Vladivostok | 1150 | 03-09-2022
- 4 | Ivanov | Warsaw | 750 | 03-04-2022
- 6 | Poirot | Berlin | 1000 | 03-01-2022
- 12 | Smith | Kyiv | 350 | 03-10-2022
- 13 | Gandi | Warsaw | 150 | 03-08-2022
-(10 rows)
-
-- Use indexscan for testing indexes after merging partitions
SET enable_seqscan = OFF;
SELECT * FROM sales_all WHERE sales_state = 'Warsaw';
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 2cdd58b2008..bf8acc5136c 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -42,6 +42,10 @@ ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, partitio
--ERROR, sales_apr_2 already exists
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_jan2022) INTO sales_apr_2;
+CREATE VIEW jan2022v as SELECT * FROM sales_jan2022;
+ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_feb2022) INTO sales_dec_jan_feb2022;
+DROP VIEW jan2022v;
+
-- NO ERROR: test for custom partitions order, source partitions not in the search_path
SET search_path = partitions_merge_schema2, public;
ALTER TABLE partitions_merge_schema.sales_range MERGE PARTITIONS (
@@ -414,9 +418,7 @@ ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_central)
-- show partitions with conditions:
EXECUTE get_partition_info('{sales_list}');
-SELECT * FROM sales_list;
-SELECT * FROM sales_nord;
-SELECT * FROM sales_all;
+SELECT tableoid::regclass, * FROM sales_list ORDER BY tableoid, salesperson_id;
-- Use indexscan for testing indexes after merging partitions
SET enable_seqscan = OFF;
--
2.34.1