Fix for dropped columns in a partitioned table's default partition

We forgot to map column numbers to/from the default partition for
various operations, leading to valid cases failing with spurious
errors, such as
ERROR:  attribute N of type some_partition has been dropped

It was also possible that the search for conflicting rows in the default
partition when attaching another partition would fail to detect some.
Secondarily, it was also possible that such a search should be skipped
(because the constraint was implied) but wasn't.

Fix all this by mapping column numbers when necessary.

Reported by: Daniel Wilches
Author: Amit Langote
Discussion: https://postgr.es/m/15873-8c61945d6b3ef87c@postgresql.org

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b60e25a0fc3bb1b9e6acbc581dff3e36c8721431..c62922a1124fdc8252b4cf31084401b593529d8f 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14973,6 +14973,13 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
                defaultrel = heap_open(defaultPartOid, NoLock);
                defPartConstraint =
                        get_proposed_default_constraint(partBoundConstraint);
+               /*
+                * Map the Vars in the constraint expression from rel's attnos to
+                * defaultrel's.
+                */
+               defPartConstraint =
+                       map_partition_varattnos(defPartConstraint,
+                                                                       1, defaultrel, rel, NULL);
                QueuePartitionConstraintValidation(wqueue, defaultrel,
                                                                                   defPartConstraint, true);
 

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 4ed99206181120171a96f7788639a173014a5fe3..128977a9112163f264506ec55dcc0786f84062a7 100644 (file)
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -609,6 +609,13 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                : get_qual_for_range(parent, new_spec, false);
        def_part_constraints =
                get_proposed_default_constraint(new_part_constraints);
+       /*
+        * Map the Vars in the constraint expression from parent's attnos to
+        * default_rel's.
+        */
+       def_part_constraints =
+                       map_partition_varattnos(def_part_constraints, 1, default_rel,
+                                                                       parent, NULL);
 
        /*
         * If the existing constraints on the default partition imply that it will
@@ -637,7 +644,6 @@ check_default_partition_contents(Relation parent, Relation default_rel,
        {
                Oid                     part_relid = lfirst_oid(lc);
                Relation        part_rel;
-               Expr       *constr;
                Expr       *partition_constraint;
                EState     *estate;
                HeapTuple       tuple;
@@ -654,6 +660,15 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                {
                        part_rel = heap_open(part_relid, NoLock);
 
+                       /*
+                        * Map the Vars in the constraint expression from default_rel's
+                        * the sub-partition's.
+                        */
+                       partition_constraint = make_ands_explicit(def_part_constraints);
+                       partition_constraint = (Expr *)
+                               map_partition_varattnos((List *) partition_constraint, 1,
+                                                                               part_rel, default_rel, NULL);
+
                        /*
                         * If the partition constraints on default partition child imply
                         * that it will not contain any row that would belong to the new
@@ -671,7 +686,10 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                        }
                }
                else
+               {
                        part_rel = default_rel;
+                       partition_constraint = make_ands_explicit(def_part_constraints);
+               }
 
                /*
                 * Only RELKIND_RELATION relations (i.e. leaf partitions) need to be
@@ -693,10 +711,6 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                }
 
                tupdesc = CreateTupleDescCopy(RelationGetDescr(part_rel));
-               constr = linitial(def_part_constraints);
-               partition_constraint = (Expr *)
-                       map_partition_varattnos((List *) constr,
-                                                                       1, part_rel, parent, NULL);
                estate = CreateExecutorState();
 
                /* Build expression execution states for partition check quals */

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 5a97b4b2a538ce2f7ae44335cba4515d84b8f84d..7c0f48d8ed4c3d84b96100fc8d60a0f2ebb8f1ad 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4109,7 +4109,8 @@ DROP USER regress_alter_table_user1;
 -- default partition
 create table defpart_attach_test (a int) partition by list (a);
 create table defpart_attach_test1 partition of defpart_attach_test for values in (1);
-create table defpart_attach_test_d (like defpart_attach_test);
+create table defpart_attach_test_d (b int, a int);
+alter table defpart_attach_test_d drop b;
 insert into defpart_attach_test_d values (1), (2);
 -- error because its constraint as the default partition would be violated
 -- by the row containing 1
@@ -4120,6 +4121,12 @@ alter table defpart_attach_test_d add check (a > 1);
 -- should be attached successfully and without needing to be scanned
 alter table defpart_attach_test attach partition defpart_attach_test_d default;
 INFO:  partition constraint for table "defpart_attach_test_d" is implied by existing constraints
+-- check that attaching a partition correctly reports any rows in the default
+-- partition that should not be there for the new partition to be attached
+-- successfully
+create table defpart_attach_test_2 (like defpart_attach_test_d);
+alter table defpart_attach_test attach partition defpart_attach_test_2 for values in (2);
+ERROR:  updated partition constraint for default partition would be violated by some row
 drop table defpart_attach_test;
 -- check combinations of temporary and permanent relations when attaching
 -- partitions.

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 41fd7ca010a9144beefcdd83504f6f9c38a39dd3..c65791d4d343ead49fdd672f9936ee8a77b83ee0 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2699,7 +2699,8 @@ DROP USER regress_alter_table_user1;
 -- default partition
 create table defpart_attach_test (a int) partition by list (a);
 create table defpart_attach_test1 partition of defpart_attach_test for values in (1);
-create table defpart_attach_test_d (like defpart_attach_test);
+create table defpart_attach_test_d (b int, a int);
+alter table defpart_attach_test_d drop b;
 insert into defpart_attach_test_d values (1), (2);
 
 -- error because its constraint as the default partition would be violated
@@ -2711,6 +2712,12 @@ alter table defpart_attach_test_d add check (a > 1);
 -- should be attached successfully and without needing to be scanned
 alter table defpart_attach_test attach partition defpart_attach_test_d default;
 
+-- check that attaching a partition correctly reports any rows in the default
+-- partition that should not be there for the new partition to be attached
+-- successfully
+create table defpart_attach_test_2 (like defpart_attach_test_d);
+alter table defpart_attach_test attach partition defpart_attach_test_2 for values in (2);
+
 drop table defpart_attach_test;
 
 -- check combinations of temporary and permanent relations when attaching