Fix bug in deciding whether to scan newly-attached partition.
authorRobert Haas
Sat, 5 Aug 2017 01:54:28 +0000 (21:54 -0400)
committerRobert Haas
Sat, 5 Aug 2017 02:01:37 +0000 (22:01 -0400)
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

src/backend/commands/tablecmds.c
src/test/regress/expected/alter_table.out
src/test/regress/sql/alter_table.sql

index 7859ef13ac4aafaf600a1923ddf3aa94b69f967b..1b8d4b3d17e87591867630d73c9cf47f5ec320a1 100644 (file)
@@ -13433,6 +13433,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
    bool        skip_validate = false;
    ObjectAddress address;
    const char *trigger_name;
+   bool        found_whole_row;
 
    attachrel = heap_openrv(cmd->name, AccessExclusiveLock);
 
@@ -13613,6 +13614,16 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
    partConstraint = (List *) canonicalize_qual((Expr *) partConstraint);
    partConstraint = list_make1(make_ands_explicit(partConstraint));
 
+   /*
+    * Adjust the generated constraint to match this partition's attribute
+    * numbers.
+    */
+   partConstraint = map_partition_varattnos(partConstraint, 1, attachrel,
+                                            rel, &found_whole_row);
+   /* There can never be a whole-row reference here */
+   if (found_whole_row)
+       elog(ERROR, "unexpected whole-row reference found in partition key");
+
    /*
     * Check if we can do away with having to scan the table being attached to
     * validate the partition constraint, by *proving* that the existing
@@ -13712,8 +13723,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
            AlteredTableInfo *tab;
            Oid         part_relid = lfirst_oid(lc);
            Relation    part_rel;
-           Expr       *constr;
-           bool        found_whole_row;
+           List       *my_partconstr = partConstraint;
 
            /* Lock already taken */
            if (part_relid != RelationGetRelid(attachrel))
@@ -13732,18 +13742,24 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
                continue;
            }
 
+           if (part_rel != attachrel)
+           {
+               /*
+                * Adjust the constraint that we constructed above for
+                * attachRel so that it matches this partition's attribute
+                * numbers.
+                */
+               my_partconstr = map_partition_varattnos(my_partconstr, 1,
+                                                       part_rel, attachrel,
+                                                       &found_whole_row);
+               /* There can never be a whole-row reference here */
+               if (found_whole_row)
+                   elog(ERROR, "unexpected whole-row reference found in partition key");
+           }
+
            /* Grab a work queue entry. */
            tab = ATGetQueueEntry(wqueue, part_rel);
-
-           /* Adjust constraint to match this partition */
-           constr = linitial(partConstraint);
-           tab->partition_constraint = (Expr *)
-               map_partition_varattnos((List *) constr, 1,
-                                       part_rel, rel,
-                                       &found_whole_row);
-           /* There can never be a whole-row reference here */
-           if (found_whole_row)
-               elog(ERROR, "unexpected whole-row reference found in partition key");
+           tab->partition_constraint = (Expr *) linitial(my_partconstr);
 
            /* keep our lock until commit */
            if (part_rel != attachrel)
index e9fd1aacce6013de574026fe0faf9437cee0f2a6..58192d2c6afacede3560d98c58dc47575558614c 100644 (file)
@@ -3347,6 +3347,51 @@ ALTER TABLE part_5 DROP CONSTRAINT check_a;
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 INFO:  partition constraint for table "part_5" is implied by existing constraints
+-- Check the case where attnos of the partitioning columns in the table being
+-- attached differs from the parent.  It should not affect the constraint-
+-- checking logic that allows to skip the scan.
+CREATE TABLE part_6 (
+   c int,
+   LIKE list_parted2,
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 6)
+);
+ALTER TABLE part_6 DROP c;
+ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
+INFO:  partition constraint for table "part_6" is implied by existing constraints
+-- Similar to above, but the table being attached is a partitioned table
+-- whose partition has still different attnos for the root partitioning
+-- columns.
+CREATE TABLE part_7 (
+   LIKE list_parted2,
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) PARTITION BY LIST (b);
+CREATE TABLE part_7_a_null (
+   c int,
+   d int,
+   e int,
+   LIKE list_parted2,  -- 'a' will have attnum = 4
+   CONSTRAINT check_b CHECK (b IS NULL OR b = 'a'),
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+);
+ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;
+ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null);
+INFO:  partition constraint for table "part_7_a_null" is implied by existing constraints
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+INFO:  partition constraint for table "part_7" is implied by existing constraints
+-- Same example, but check this time that the constraint correctly detects
+-- violating rows
+ALTER TABLE list_parted2 DETACH PARTITION part_7;
+ALTER TABLE part_7 DROP CONSTRAINT check_a; -- thusly, scan won't be skipped
+INSERT INTO part_7 (a, b) VALUES (8, null), (9, 'a');
+SELECT tableoid::regclass, a, b FROM part_7 order by a;
+   tableoid    | a | b 
+---------------+---+---
+ part_7_a_null | 8 | 
+ part_7_a_null | 9 | a
+(2 rows)
+
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+ERROR:  partition constraint is violated by some row
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);
 ERROR:  "part_2" is already a partition
index 5dd1402ea6f2137293af4d92a3268b3df184dc24..9a20dd141a2543df9749ea6d29d30d7152e0bf10 100644 (file)
@@ -2178,6 +2178,44 @@ ALTER TABLE part_5 DROP CONSTRAINT check_a;
 ALTER TABLE part_5 ADD CONSTRAINT check_a CHECK (a IN (5)), ALTER a SET NOT NULL;
 ALTER TABLE list_parted2 ATTACH PARTITION part_5 FOR VALUES IN (5);
 
+-- Check the case where attnos of the partitioning columns in the table being
+-- attached differs from the parent.  It should not affect the constraint-
+-- checking logic that allows to skip the scan.
+CREATE TABLE part_6 (
+   c int,
+   LIKE list_parted2,
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 6)
+);
+ALTER TABLE part_6 DROP c;
+ALTER TABLE list_parted2 ATTACH PARTITION part_6 FOR VALUES IN (6);
+
+-- Similar to above, but the table being attached is a partitioned table
+-- whose partition has still different attnos for the root partitioning
+-- columns.
+CREATE TABLE part_7 (
+   LIKE list_parted2,
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+) PARTITION BY LIST (b);
+CREATE TABLE part_7_a_null (
+   c int,
+   d int,
+   e int,
+   LIKE list_parted2,  -- 'a' will have attnum = 4
+   CONSTRAINT check_b CHECK (b IS NULL OR b = 'a'),
+   CONSTRAINT check_a CHECK (a IS NOT NULL AND a = 7)
+);
+ALTER TABLE part_7_a_null DROP c, DROP d, DROP e;
+ALTER TABLE part_7 ATTACH PARTITION part_7_a_null FOR VALUES IN ('a', null);
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+
+-- Same example, but check this time that the constraint correctly detects
+-- violating rows
+ALTER TABLE list_parted2 DETACH PARTITION part_7;
+ALTER TABLE part_7 DROP CONSTRAINT check_a; -- thusly, scan won't be skipped
+INSERT INTO part_7 (a, b) VALUES (8, null), (9, 'a');
+SELECT tableoid::regclass, a, b FROM part_7 order by a;
+ALTER TABLE list_parted2 ATTACH PARTITION part_7 FOR VALUES IN (7);
+
 -- check that the table being attached is not already a partition
 ALTER TABLE list_parted2 ATTACH PARTITION part_2 FOR VALUES IN (2);