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

Author: robertmhaas

src/backend/commands/tablecmds.c

@@ -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)

src/test/regress/expected/alter_table.out

@@ -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

src/test/regress/sql/alter_table.sql

@@ -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);