Skip to content

Navigation Menu

Sign in
Appearance settings

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 f85f88b

Browse filesBrowse files
robertmhaasrobertmhaas
committed
Fix bug in deciding whether to scan newly-attached partition.
If the table being attached had different attribute numbers than the parent, the old code could incorrectly decide it needed to be scanned. Amit Langote, reviewed by Ashutosh Bapat Discussion: http://postgr.es/m/CA+TgmobexgbBr2+Utw-pOMw9uxaBRKRjMW_-mmzKKx9PejPLMg@mail.gmail.com
1 parent 7e174fa commit f85f88b
Copy full SHA for f85f88b

File tree

Expand file treeCollapse file tree

3 files changed

+111
-12
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+111
-12
lines changed

‎src/backend/commands/tablecmds.c

Copy file name to clipboardExpand all lines: src/backend/commands/tablecmds.c
+28-12Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13433,6 +13433,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1343313433
bool skip_validate = false;
1343413434
ObjectAddress address;
1343513435
const char *trigger_name;
13436+
bool found_whole_row;
1343613437

1343713438
attachrel = heap_openrv(cmd->name, AccessExclusiveLock);
1343813439

@@ -13613,6 +13614,16 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1361313614
partConstraint = (List *) canonicalize_qual((Expr *) partConstraint);
1361413615
partConstraint = list_make1(make_ands_explicit(partConstraint));
1361513616

13617+
/*
13618+
* Adjust the generated constraint to match this partition's attribute
13619+
* numbers.
13620+
*/
13621+
partConstraint = map_partition_varattnos(partConstraint, 1, attachrel,
13622+
rel, &found_whole_row);
13623+
/* There can never be a whole-row reference here */
13624+
if (found_whole_row)
13625+
elog(ERROR, "unexpected whole-row reference found in partition key");
13626+
1361613627
/*
1361713628
* Check if we can do away with having to scan the table being attached to
1361813629
* validate the partition constraint, by *proving* that the existing
@@ -13712,8 +13723,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1371213723
AlteredTableInfo *tab;
1371313724
Oid part_relid = lfirst_oid(lc);
1371413725
Relation part_rel;
13715-
Expr *constr;
13716-
bool found_whole_row;
13726+
List *my_partconstr = partConstraint;
1371713727

1371813728
/* Lock already taken */
1371913729
if (part_relid != RelationGetRelid(attachrel))
@@ -13732,18 +13742,24 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
1373213742
continue;
1373313743
}
1373413744

13745+
if (part_rel != attachrel)
13746+
{
13747+
/*
13748+
* Adjust the constraint that we constructed above for
13749+
* attachRel so that it matches this partition's attribute
13750+
* numbers.
13751+
*/
13752+
my_partconstr = map_partition_varattnos(my_partconstr, 1,
13753+
part_rel, attachrel,
13754+
&found_whole_row);
13755+
/* There can never be a whole-row reference here */
13756+
if (found_whole_row)
13757+
elog(ERROR, "unexpected whole-row reference found in partition key");
13758+
}
13759+
1373513760
/* Grab a work queue entry. */
1373613761
tab = ATGetQueueEntry(wqueue, part_rel);
13737-
13738-
/* Adjust constraint to match this partition */
13739-
constr = linitial(partConstraint);
13740-
tab->partition_constraint = (Expr *)
13741-
map_partition_varattnos((List *) constr, 1,
13742-
part_rel, rel,
13743-
&found_whole_row);
13744-
/* There can never be a whole-row reference here */
13745-
if (found_whole_row)
13746-
elog(ERROR, "unexpected whole-row reference found in partition key");
13762+
tab->partition_constraint = (Expr *) linitial(my_partconstr);
1374713763

1374813764
/* keep our lock until commit */
1374913765
if (part_rel != attachrel)

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

Copy file name to clipboardExpand all lines: src/test/regress/expected/alter_table.out
+45Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3347,6 +3347,51 @@ ALTER TABLE part_5 DROP CONSTRAINT check_a;
33473347
ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
33483348
ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
33493349
INFO: partition constraint for table "part_5" is implied by existing constraints
3350+
-- Check the case where attnos of the partitioning columns in the table being
3351+
-- attached differs from the parent. It should not affect the constraint-
3352+
-- checking logic that allows to skip the scan.
3353+
CREATE TABLE part_6 (
3354+
c int,
3355+
LIKE list_parted2,
3356+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 6)
3357+
);
3358+
ALTER TABLE part_6 DROP c;
3359+
ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
3360+
INFO: partition constraint for table "part_6" is implied by existing constraints
3361+
-- Similar to above, but the table being attached is a partitioned table
3362+
-- whose partition has still different attnos for the root partitioning
3363+
-- columns.
3364+
CREATE TABLE part_7 (
3365+
LIKE list_parted2,
3366+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
3367+
) PARTITION BY LIST (b);
3368+
CREATE TABLE part_7_a_null (
3369+
c int,
3370+
d int,
3371+
e int,
3372+
LIKE list_parted2, -- 'a' will have attnum = 4
3373+
CONSTRAINT check_b CHECK (b IS NULL OR b = 'a'),
3374+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
3375+
);
3376+
ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;
3377+
ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null);
3378+
INFO: partition constraint for table "part_7_a_null" is implied by existing constraints
3379+
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
3380+
INFO: partition constraint for table "part_7" is implied by existing constraints
3381+
-- Same example, but check this time that the constraint correctly detects
3382+
-- violating rows
3383+
ALTER TABLE list_parted2 DETACH PARTITION part_7;
3384+
ALTER TABLE part_7 DROP CONSTRAINT check_a; -- thusly, scan won't be skipped
3385+
INSERT INTO part_7 (a, b) VALUES (8, null), (9, 'a');
3386+
SELECT tableoid::regclass, a, b FROM part_7 order by a;
3387+
tableoid | a | b
3388+
---------------+---+---
3389+
part_7_a_null | 8 |
3390+
part_7_a_null | 9 | a
3391+
(2 rows)
3392+
3393+
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
3394+
ERROR: partition constraint is violated by some row
33503395
-- check that the table being attached is not already a partition
33513396
ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
33523397
ERROR: "part_2" is already a partition

‎src/test/regress/sql/alter_table.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/alter_table.sql
+38Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,6 +2178,44 @@ ALTER TABLE part_5 DROP CONSTRAINT check_a;
21782178
ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
21792179
ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
21802180

2181+
-- Check the case where attnos of the partitioning columns in the table being
2182+
-- attached differs from the parent. It should not affect the constraint-
2183+
-- checking logic that allows to skip the scan.
2184+
CREATE TABLE part_6 (
2185+
c int,
2186+
LIKE list_parted2,
2187+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 6)
2188+
);
2189+
ALTER TABLE part_6 DROP c;
2190+
ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
2191+
2192+
-- Similar to above, but the table being attached is a partitioned table
2193+
-- whose partition has still different attnos for the root partitioning
2194+
-- columns.
2195+
CREATE TABLE part_7 (
2196+
LIKE list_parted2,
2197+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
2198+
) PARTITION BY LIST (b);
2199+
CREATE TABLE part_7_a_null (
2200+
c int,
2201+
d int,
2202+
e int,
2203+
LIKE list_parted2, -- 'a' will have attnum = 4
2204+
CONSTRAINT check_b CHECK (b IS NULL OR b = 'a'),
2205+
CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
2206+
);
2207+
ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;
2208+
ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null);
2209+
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
2210+
2211+
-- Same example, but check this time that the constraint correctly detects
2212+
-- violating rows
2213+
ALTER TABLE list_parted2 DETACH PARTITION part_7;
2214+
ALTER TABLE part_7 DROP CONSTRAINT check_a; -- thusly, scan won't be skipped
2215+
INSERT INTO part_7 (a, b) VALUES (8, null), (9, 'a');
2216+
SELECT tableoid::regclass, a, b FROM part_7 order by a;
2217+
ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
2218+
21812219
-- check that the table being attached is not already a partition
21822220
ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
21832221

0 commit comments

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