]> granicus.if.org Git - postgresql/commitdiff
Fix for dropped columns in a partitioned table's default partition
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 28 Jun 2019 18:51:08 +0000 (14:51 -0400)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 28 Jun 2019 18:51:08 +0000 (14:51 -0400)
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

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

index 7dcd634a1aea9c99d494a75489504faa610f36a2..fd67d2a841378e039502a544b6405d0dcababcb9 100644 (file)
@@ -15682,6 +15682,13 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
                defaultrel = table_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);
 
index 99d26de7e648e745bd351ac7dcdacbe05591ede2..f3c9236ad56f2f2b9fbd94eaf03a9b10d6531292 100644 (file)
@@ -1237,6 +1237,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
@@ -1265,7 +1272,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;
                ExprState  *partqualstate = NULL;
@@ -1280,6 +1286,15 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                {
                        part_rel = table_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
@@ -1297,7 +1312,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
@@ -1318,10 +1336,6 @@ check_default_partition_contents(Relation parent, Relation default_rel,
                        continue;
                }
 
-               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 */
index a639601f66659489939bdb14a4ca94e6f1fa753f..e687150511bb09f7e965ca95289f2c6c2b8f55d1 100644 (file)
@@ -4008,7 +4008,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
@@ -4019,6 +4020,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.
index 204441e7596dfc5993d6f804b22872aaa63e5d85..262abf2bfb81b98f7bfa739d0135fbe38b07d093 100644 (file)
@@ -1119,3 +1119,17 @@ select tableoid::regclass from volatile_partbound_test;
 (1 row)
 
 drop table volatile_partbound_test;
+-- test the case where a check constraint on default partition allows
+-- to avoid scanning it when adding a new partition
+create table defcheck (a int, b int) partition by list (b);
+create table defcheck_def (a int, c int, b int);
+alter table defcheck_def drop c;
+alter table defcheck attach partition defcheck_def default;
+alter table defcheck_def add check (b <= 0 and b is not null);
+create table defcheck_1 partition of defcheck for values in (1, null);
+INFO:  updated partition constraint for default partition "defcheck_def" is implied by existing constraints
+-- test that complex default partition constraints are enforced correctly
+insert into defcheck_def values (0, 0);
+create table defcheck_0 partition of defcheck for values in (0);
+ERROR:  updated partition constraint for default partition "defcheck_def" would be violated by some row
+drop table defcheck;
index 0369909b508d8f04d3b7939999785075c71c868b..8016f8a823a794c69b2459167d288e9828c5f66d 100644 (file)
@@ -2648,7 +2648,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
@@ -2660,6 +2661,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
index 751c0d39f5d30ad7ac366be4519c3705da818f65..9c6d86a0bfd19054c50555274772bc57f29ec898 100644 (file)
@@ -856,3 +856,17 @@ create table volatile_partbound_test2 partition of volatile_partbound_test for v
 insert into volatile_partbound_test values (current_timestamp);
 select tableoid::regclass from volatile_partbound_test;
 drop table volatile_partbound_test;
+
+-- test the case where a check constraint on default partition allows
+-- to avoid scanning it when adding a new partition
+create table defcheck (a int, b int) partition by list (b);
+create table defcheck_def (a int, c int, b int);
+alter table defcheck_def drop c;
+alter table defcheck attach partition defcheck_def default;
+alter table defcheck_def add check (b <= 0 and b is not null);
+create table defcheck_1 partition of defcheck for values in (1, null);
+
+-- test that complex default partition constraints are enforced correctly
+insert into defcheck_def values (0, 0);
+create table defcheck_0 partition of defcheck for values in (0);
+drop table defcheck;