From 1f8a3327a9db9a8a662fb39fdcde2337acffa68c Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 12 Mar 2018 19:42:32 -0300 Subject: [PATCH] Avoid having two PKs in a partition If a table containing a primary key is attach as partition to a partitioned table which has a primary key with a different definition, we would happily create a second one in the new partition. Oops. It turns out that this is because an error check in DefineIndex is executed only if you tell it that it's being run by ALTER TABLE, and the original code here wasn't. Change it so that it does. Added a couple of test cases for this, also. A previously working test started to fail in a different way than before patch because the new check is called earlier; change the PK to plain UNIQUE so that the new behavior isn't invoked, so that the test continues to verify what we want it to verify. Reported by: Noriyoshi Shinoda Discussion: https://postgr.es/m/DF4PR8401MB102060EC2615EC9227CC73F7EEDF0@DF4PR8401MB1020.NAMPRD84.PROD.OUTLOOK.COM --- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/tablecmds.c | 2 +- src/test/regress/expected/indexing.out | 26 +++++++++++++++++++++----- src/test/regress/sql/indexing.sql | 22 +++++++++++++++++++--- 4 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 504806b25b..ff40683d06 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -973,7 +973,7 @@ DefineIndex(Oid relationId, InvalidOid, /* no predefined OID */ indexRelationId, /* this is our child */ createdConstraintId, - false, check_rights, check_not_in_use, + is_alter_table, check_rights, check_not_in_use, false, quiet); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 46a648a33a..218224a156 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14256,7 +14256,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) DefineIndex(RelationGetRelid(attachrel), stmt, InvalidOid, RelationGetRelid(idxRel), constraintOid, - false, false, false, false, false); + true, false, false, false, false); } index_close(idxRel, AccessShareLock); diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 3fa47cba96..fb32ffffdc 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -878,7 +878,7 @@ select conname, contype, conrelid::regclass, conindid::regclass, conkey drop table idxpart; -- Verify that multi-layer partitioning honors the requirement that all --- columns in the partition key must appear in primary key +-- columns in the partition key must appear in primary/unique key create table idxpart (a int, b int, primary key (a)) partition by range (a); create table idxpart2 partition of idxpart for values from (0) to (1000) partition by range (b); -- fail @@ -886,12 +886,12 @@ ERROR: insufficient columns in PRIMARY KEY constraint definition DETAIL: PRIMARY KEY constraint on table "idxpart2" lacks column "b" which is part of the partition key. drop table idxpart; -- Ditto for the ATTACH PARTITION case -create table idxpart (a int primary key, b int) partition by range (a); -create table idxpart1 (a int not null, b int, primary key (a, b)) +create table idxpart (a int unique, b int) partition by range (a); +create table idxpart1 (a int not null, b int, unique (a, b)) partition by range (a, b); alter table idxpart attach partition idxpart1 for values from (1) to (1000); -ERROR: insufficient columns in PRIMARY KEY constraint definition -DETAIL: PRIMARY KEY constraint on table "idxpart1" lacks column "b" which is part of the partition key. +ERROR: insufficient columns in UNIQUE constraint definition +DETAIL: UNIQUE constraint on table "idxpart1" lacks column "b" which is part of the partition key. DROP TABLE idxpart, idxpart1; -- Multi-layer partitioning works correctly in this case: create table idxpart (a int, b int, primary key (a, b)) partition by range (a); @@ -952,6 +952,22 @@ select indrelid::regclass, indexrelid::regclass, inhparent::regclass, indisvalid (0 rows) drop table idxpart; +-- If the partition to be attached already has a primary key, fail if +-- it doesn't match the parent's PK. +CREATE TABLE idxpart (c1 INT PRIMARY KEY, c2 INT, c3 VARCHAR(10)) PARTITION BY RANGE(c1); +CREATE TABLE idxpart1 (LIKE idxpart); +ALTER TABLE idxpart1 ADD PRIMARY KEY (c1, c2); +ALTER TABLE idxpart ATTACH PARTITION idxpart1 FOR VALUES FROM (100) TO (200); +ERROR: multiple primary keys for table "idxpart1" are not allowed +DROP TABLE idxpart, idxpart1; +-- Ditto if there is some distance between the PKs (subpartitioning) +create table idxpart (a int, b int, primary key (a)) partition by range (a); +create table idxpart1 (a int not null, b int) partition by range (a); +create table idxpart11 (a int not null, b int primary key); +alter table idxpart1 attach partition idxpart11 for values from (0) to (1000); +alter table idxpart attach partition idxpart1 for values from (0) to (10000); +ERROR: multiple primary keys for table "idxpart11" are not allowed +drop table idxpart, idxpart1, idxpart11; -- If a partitioned table has a constraint whose index is not valid, -- attaching a missing partition makes it valid. create table idxpart (a int) partition by range (a); diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 902d8c59ca..c4ab89fc48 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -444,15 +444,15 @@ select conname, contype, conrelid::regclass, conindid::regclass, conkey drop table idxpart; -- Verify that multi-layer partitioning honors the requirement that all --- columns in the partition key must appear in primary key +-- columns in the partition key must appear in primary/unique key create table idxpart (a int, b int, primary key (a)) partition by range (a); create table idxpart2 partition of idxpart for values from (0) to (1000) partition by range (b); -- fail drop table idxpart; -- Ditto for the ATTACH PARTITION case -create table idxpart (a int primary key, b int) partition by range (a); -create table idxpart1 (a int not null, b int, primary key (a, b)) +create table idxpart (a int unique, b int) partition by range (a); +create table idxpart1 (a int not null, b int, unique (a, b)) partition by range (a, b); alter table idxpart attach partition idxpart1 for values from (1) to (1000); DROP TABLE idxpart, idxpart1; @@ -494,6 +494,22 @@ select indrelid::regclass, indexrelid::regclass, inhparent::regclass, indisvalid order by indexrelid::regclass::text collate "C"; drop table idxpart; +-- If the partition to be attached already has a primary key, fail if +-- it doesn't match the parent's PK. +CREATE TABLE idxpart (c1 INT PRIMARY KEY, c2 INT, c3 VARCHAR(10)) PARTITION BY RANGE(c1); +CREATE TABLE idxpart1 (LIKE idxpart); +ALTER TABLE idxpart1 ADD PRIMARY KEY (c1, c2); +ALTER TABLE idxpart ATTACH PARTITION idxpart1 FOR VALUES FROM (100) TO (200); +DROP TABLE idxpart, idxpart1; + +-- Ditto if there is some distance between the PKs (subpartitioning) +create table idxpart (a int, b int, primary key (a)) partition by range (a); +create table idxpart1 (a int not null, b int) partition by range (a); +create table idxpart11 (a int not null, b int primary key); +alter table idxpart1 attach partition idxpart11 for values from (0) to (1000); +alter table idxpart attach partition idxpart1 for values from (0) to (10000); +drop table idxpart, idxpart1, idxpart11; + -- If a partitioned table has a constraint whose index is not valid, -- attaching a missing partition makes it valid. create table idxpart (a int) partition by range (a);