Fix assertion failure in check_new_partition_bound().

Commit 6b2c4e59d was overly confident about not being able to see
a negative cmpval result from partition_range_bsearch().  Adjust
the code to cope with that.

Report and patch by Amul Sul; some additional cosmetic changes by me

Discussion: https://postgr.es/m/CAAJ_b97WCO=EyVA7fKzc86kKfojHXLU04_zs7-7+yVzm=-1QkQ@mail.gmail.com

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 66c42b58986e6916b4fd1b2c101e3c2a5feb1c78..a4f97c10fe7a79e832dd2bfef78e3fa9e831500e 100644 (file)
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -2703,10 +2703,10 @@ add_merged_range_bounds(int partnatts, FmgrInfo *partsupfuncs,
        prev_ub.lower = false;
 
        /*
-        * We pass to partition_rbound_cmp() lower1 as false to prevent it
-        * from considering the last upper bound to be smaller than the lower
-        * bound of the merged partition when the values of the two range
-        * bounds compare equal.
+        * We pass lower1 = false to partition_rbound_cmp() to prevent it from
+        * considering the last upper bound to be smaller than the lower bound
+        * of the merged partition when the values of the two range bounds
+        * compare equal.
         */
        cmpval = partition_rbound_cmp(partnatts, partsupfuncs, partcollations,
                                      merged_lb->datums, merged_lb->kind,
@@ -2978,16 +2978,19 @@ check_new_partition_bound(char *relname, Relation parent,
 
                /*
                 * First check if the resulting range would be empty with
-                * specified lower and upper bounds
+                * specified lower and upper bounds.  partition_rbound_cmp
+                * cannot return zero here, since the lower-bound flags are
+                * different.
                 */
                cmpval = partition_rbound_cmp(key->partnatts,
                                              key->partsupfunc,
                                              key->partcollation,
                                              lower->datums, lower->kind,
                                              true, upper);
-               if (cmpval >= 0)
+               Assert(cmpval != 0);
+               if (cmpval > 0)
                {
-                   /* Fetch the problematic key from the lower datums list. */
+                   /* Point to problematic key in the lower datums list. */
                    PartitionRangeDatum *datum = list_nth(spec->lowerdatums,
                                                          cmpval - 1);
 
@@ -3057,11 +3060,11 @@ check_new_partition_bound(char *relname, Relation parent,
                            if (cmpval < 0)
                            {
                                /*
-                                * Fetch the problematic key from the upper
+                                * Point to problematic key in the upper
                                 * datums list.
                                 */
                                PartitionRangeDatum *datum =
-                               list_nth(spec->upperdatums, -cmpval - 1);
+                               list_nth(spec->upperdatums, Abs(cmpval) - 1);
 
                                /*
                                 * The new partition overlaps with the
@@ -3083,15 +3086,11 @@ check_new_partition_bound(char *relname, Relation parent,
                        PartitionRangeDatum *datum;
 
                        /*
-                        * Fetch the problematic key from the lower datums
-                        * list.  Given the way partition_range_bsearch()
-                        * works, the new lower bound is certainly >= the
-                        * bound at offset.  If the bound matches exactly, we
-                        * flag the 1st key.
+                        * Point to problematic key in the lower datums list;
+                        * if we have equality, point to the first one.
                         */
-                       Assert(cmpval >= 0);
                        datum = cmpval == 0 ? linitial(spec->lowerdatums) :
-                           list_nth(spec->lowerdatums, cmpval - 1);
+                           list_nth(spec->lowerdatums, Abs(cmpval) - 1);
                        overlap = true;
                        overlap_location = datum->location;
                        with = boundinfo->indexes[offset + 1];
@@ -3393,13 +3392,14 @@ partition_rbound_cmp(int partnatts, FmgrInfo *partsupfunc,
        else if (kind1[i] > kind2[i])
            return colnum;
        else if (kind1[i] != PARTITION_RANGE_DATUM_VALUE)
-
+       {
            /*
             * The column bounds are both MINVALUE or both MAXVALUE. No later
             * columns should be considered, but we still need to compare
             * whether they are upper or lower bounds.
             */
            break;
+       }
 
        cmpval = DatumGetInt32(FunctionCall2Coll(&partsupfunc[i],
                                                 partcollation[i],
@@ -3692,9 +3692,9 @@ qsort_partition_rbound_cmp(const void *a, const void *b, void *arg)
    PartitionRangeBound *b2 = (*(PartitionRangeBound *const *) b);
    PartitionKey key = (PartitionKey) arg;
 
-   return partition_rbound_cmp(key->partnatts, key->partsupfunc,
-                               key->partcollation, b1->datums, b1->kind,
-                               b1->lower, b2);
+   return compare_range_bounds(key->partnatts, key->partsupfunc,
+                               key->partcollation,
+                               b1, b2);
 }
 
 /*

diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 1fc266dd65cbe153bd7f34d08206f20aa5257530..ed8c01b8decc99e4012e6cc6eaa7fb0305cc8d33 100644 (file)
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -856,6 +856,10 @@ ERROR:  partition "fail_part" would overlap partition "part0"
 LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) ...
                                                              ^
 CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10);
+CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1);
+ERROR:  partition "fail_part" would overlap partition "part0"
+LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1)...
+                                                             ^
 CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (maxvalue);
 ERROR:  partition "fail_part" would overlap partition "part1"
 LINE 1: ..._part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (max...

diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index cee822aa8b6eeb6909194ee587c6490310df93b3..d257679ba68c1ec27b55fd71eb468d62f0633c49 100644 (file)
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -687,6 +687,7 @@ CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (1) TO (1);
 CREATE TABLE part0 PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (1);
 CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (minvalue) TO (2);
 CREATE TABLE part1 PARTITION OF range_parted2 FOR VALUES FROM (1) TO (10);
+CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (-1) TO (1);
 CREATE TABLE fail_part PARTITION OF range_parted2 FOR VALUES FROM (9) TO (maxvalue);
 CREATE TABLE part2 PARTITION OF range_parted2 FOR VALUES FROM (20) TO (30);
 CREATE TABLE part3 PARTITION OF range_parted2 FOR VALUES FROM (30) TO (40);