Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3a82c68

Browse filesBrowse files
akorotkovakorotkov
committed
Fix the name collision detection in MERGE/SPLIT partition operations
Both MERGE and SPLIT partition operations support the case when the name of the new partition matches the name of the existing partition to be merged/split. But the name collision detection doesn't always work as intended. The SPLIT partition operation finds the namespace to search for an existing partition without taking into account the parent's persistence. The MERGE partition operation checks for the name collision with simple equal() on RangeVar's simply ignoring the search_path. This commit fixes this behavior as follows. 1. The SPLIT partition operation now finds the namespace to search for an existing partition according to the parent's persistence. 2. The MERGE partition operation now checks for the name collision similarly to the SPLIT partition operation using RangeVarGetAndCheckCreationNamespace() and get_relname_relid(). Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com Author: Dmitry Koval, Alexander Korotkov
1 parent 97e5b00 commit 3a82c68
Copy full SHA for 3a82c68

File tree

5 files changed

+72
-13
lines changed
Filter options

5 files changed

+72
-13
lines changed

‎src/backend/commands/tablecmds.c

Copy file name to clipboardExpand all lines: src/backend/commands/tablecmds.c
+51-11Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20409,6 +20409,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
2040920409
* against concurrent drop, and mark stmt->relation as
2041020410
* RELPERSISTENCE_TEMP if a temporary namespace is selected.
2041120411
*/
20412+
sps->name->relpersistence = rel->rd_rel->relpersistence;
2041220413
namespaceId =
2041320414
RangeVarGetAndCheckCreationNamespace(sps->name, NoLock, NULL);
2041420415

@@ -20601,6 +20602,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
2060120602
ListCell *listptr;
2060220603
List *mergingPartitionsList = NIL;
2060320604
Oid defaultPartOid;
20605+
Oid namespaceId;
20606+
Oid existingRelid;
2060420607

2060520608
/*
2060620609
* Lock all merged partitions, check them and create list with partitions
@@ -20617,13 +20620,48 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
2061720620
*/
2061820621
mergingPartition = table_openrv(name, AccessExclusiveLock);
2061920622

20620-
/*
20621-
* Checking that two partitions have the same name was before, in
20622-
* function transformPartitionCmdForMerge().
20623-
*/
20624-
if (equal(name, cmd->name))
20623+
/* Store a next merging partition into the list. */
20624+
mergingPartitionsList = lappend(mergingPartitionsList,
20625+
mergingPartition);
20626+
}
20627+
20628+
/*
20629+
* Look up the namespace in which we are supposed to create the partition,
20630+
* check we have permission to create there, lock it against concurrent
20631+
* drop, and mark stmt->relation as RELPERSISTENCE_TEMP if a temporary
20632+
* namespace is selected.
20633+
*/
20634+
cmd->name->relpersistence = rel->rd_rel->relpersistence;
20635+
namespaceId =
20636+
RangeVarGetAndCheckCreationNamespace(cmd->name, NoLock, NULL);
20637+
20638+
/*
20639+
* Check if this name is already taken. This helps us to detect the
20640+
* situation when one of the merging partitions has the same name as the
20641+
* new partition. Otherwise, this would fail later on anyway but catching
20642+
* this here allows us to emit a nicer error message.
20643+
*/
20644+
existingRelid = get_relname_relid(cmd->name->relname, namespaceId);
20645+
20646+
if (OidIsValid(existingRelid))
20647+
{
20648+
Relation sameNamePartition = NULL;
20649+
20650+
foreach_ptr(RelationData, mergingPartition, mergingPartitionsList)
2062520651
{
20626-
/* One new partition can have the same name as merged partition. */
20652+
if (RelationGetRelid(mergingPartition) == existingRelid)
20653+
{
20654+
sameNamePartition = mergingPartition;
20655+
break;
20656+
}
20657+
}
20658+
20659+
if (sameNamePartition)
20660+
{
20661+
/*
20662+
* The new partition has the same name as one of merging
20663+
* partitions.
20664+
*/
2062720665
char tmpRelName[NAMEDATALEN];
2062820666

2062920667
/* Generate temporary name. */
@@ -20635,7 +20673,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
2063520673
* in the future because we're going to eventually drop the
2063620674
* existing partition anyway.
2063720675
*/
20638-
RenameRelationInternal(RelationGetRelid(mergingPartition),
20676+
RenameRelationInternal(RelationGetRelid(sameNamePartition),
2063920677
tmpRelName, false, false);
2064020678

2064120679
/*
@@ -20644,10 +20682,12 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
2064420682
*/
2064520683
CommandCounterIncrement();
2064620684
}
20647-
20648-
/* Store a next merging partition into the list. */
20649-
mergingPartitionsList = lappend(mergingPartitionsList,
20650-
mergingPartition);
20685+
else
20686+
{
20687+
ereport(ERROR,
20688+
(errcode(ERRCODE_DUPLICATE_TABLE),
20689+
errmsg("relation \"%s\" already exists", cmd->name->relname)));
20690+
}
2065120691
}
2065220692

2065320693
/* Detach all merged partitions. */

‎src/test/regress/expected/partition_merge.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/partition_merge.out
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ INSERT INTO sales_range VALUES (13, 'Gandi', 377, '2022-01-09');
214214
INSERT INTO sales_range VALUES (14, 'Smith', 510, '2022-05-04');
215215
-- Merge partitions (include DEFAULT partition) into partition with the same
216216
-- name
217-
ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
217+
ALTER TABLE sales_range MERGE PARTITIONS
218+
(sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
218219
select * from sales_others;
219220
salesperson_id | salesperson_name | sales_amount | sales_date
220221
----------------+------------------+--------------+------------

‎src/test/regress/expected/partition_split.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/partition_split.out
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,6 +1547,14 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_alice;
15471547
REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
15481548
DROP ROLE regress_partition_split_alice;
15491549
DROP ROLE regress_partition_split_bob;
1550+
-- Split partition of a temporary table when one of the partitions after
1551+
-- split has the same name as the partition being split
1552+
CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
1553+
CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
1554+
ALTER TABLE t SPLIT PARTITION tp_0 INTO
1555+
(PARTITION tp_0 FOR VALUES FROM (0) TO (1),
1556+
PARTITION tp_1 FOR VALUES FROM (1) TO (2));
1557+
DROP TABLE t;
15501558
RESET search_path;
15511559
--
15521560
DROP SCHEMA partition_split_schema;

‎src/test/regress/sql/partition_merge.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/partition_merge.sql
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ INSERT INTO sales_range VALUES (14, 'Smith', 510, '2022-05-04');
140140

141141
-- Merge partitions (include DEFAULT partition) into partition with the same
142142
-- name
143-
ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022, sales_others) INTO sales_others;
143+
ALTER TABLE sales_range MERGE PARTITIONS
144+
(sales_jan2022, sales_mar2022, partitions_merge_schema.sales_others) INTO sales_others;
144145

145146
select * from sales_others;
146147

‎src/test/regress/sql/partition_split.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/partition_split.sql
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,15 @@ REVOKE ALL ON SCHEMA partition_split_schema FROM regress_partition_split_bob;
931931
DROP ROLE regress_partition_split_alice;
932932
DROP ROLE regress_partition_split_bob;
933933

934+
-- Split partition of a temporary table when one of the partitions after
935+
-- split has the same name as the partition being split
936+
CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a);
937+
CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2);
938+
ALTER TABLE t SPLIT PARTITION tp_0 INTO
939+
(PARTITION tp_0 FOR VALUES FROM (0) TO (1),
940+
PARTITION tp_1 FOR VALUES FROM (1) TO (2));
941+
DROP TABLE t;
942+
934943
RESET search_path;
935944

936945
--

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.