Clarify the contract of partition_rbound_cmp().

partition_rbound_cmp() is intended to compare range partition bounds
in a way such that if all the bound values are equal but one is an
upper bound and one is a lower bound, the upper bound is treated as
smaller than the lower bound. This particular ordering is required by
RelationBuildPartitionDesc() when building the PartitionBoundInfoData,
so that it can consistently keep only the upper bounds when upper and
lower bounds coincide.

Update the function comment to make that clearer.

Also, fix a (currently unreachable) corner-case bug -- if the bound
values coincide and they contain unbounded values, fall through to the
lower-vs-upper comparison code, rather than immediately returning
0. Currently it is not possible to define coincident upper and lower
bounds containing unbounded columns, but that may change in the
future, so code defensively.

Discussion: https://postgr.es/m/CAAJ_b947mowpLdxL3jo3YLKngRjrq9+Ej4ymduQTfYR+8=YAYQ@mail.gmail.com

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 955c3497b7b2f23a060bf9f68b3a05091cb128b4..43b892426118cbf2072e2bb56433adb07f2c626a 100644 (file)
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -57,7 +57,8 @@
  *
  * In the case of range partitioning, ndatums will typically be far less than
  * 2 * nparts, because a partition's upper bound and the next partition's lower
- * bound are the same in most common cases, and we only store one of them.
+ * bound are the same in most common cases, and we only store one of them (the
+ * upper bound).
  *
  * In the case of list partitioning, the indexes array stores one entry for
  * every datum, which is the index of the partition that accepts a given datum.
@@ -2136,7 +2137,14 @@ qsort_partition_rbound_cmp(const void *a, const void *b, void *arg)
  * partition_rbound_cmp
  *
  * Return for two range bounds whether the 1st one (specified in datum1,
- * content1, and lower1) is <=, =, >= the bound specified in *b2
+ * content1, and lower1) is <, =, or > the bound specified in *b2.
+ *
+ * Note that if the values of the two range bounds compare equal, then we take
+ * into account whether they are upper or lower bounds, and an upper bound is
+ * considered to be smaller than a lower bound. This is important to the way
+ * that RelationBuildPartitionDesc() builds the PartitionBoundInfoData
+ * structure, which only stores the upper bound of a common boundary between
+ * two contiguous partitions.
  */
 static int32
 partition_rbound_cmp(PartitionKey key,
@@ -2152,22 +2160,30 @@ partition_rbound_cmp(PartitionKey key,
        for (i = 0; i < key->partnatts; i++)
        {
                /*
-                * First, handle cases involving infinity, which don't require
-                * invoking the comparison proc.
+                * First, handle cases where the column is unbounded, which should not
+                * invoke the comparison procedure, and should not consider any later
+                * columns.
                 */
-               if (content1[i] != RANGE_DATUM_FINITE &&
+               if (content1[i] != RANGE_DATUM_FINITE ||
                        content2[i] != RANGE_DATUM_FINITE)
-
+               {
                        /*
-                        * Both are infinity, so they are equal unless one is negative
-                        * infinity and other positive (or vice versa)
+                        * If the bound values are equal, fall through and compare whether
+                        * they are upper or lower bounds.
                         */
-                       return content1[i] == content2[i] ? 0
-                               : (content1[i] < content2[i] ? -1 : 1);
-               else if (content1[i] != RANGE_DATUM_FINITE)
-                       return content1[i] == RANGE_DATUM_NEG_INF ? -1 : 1;
-               else if (content2[i] != RANGE_DATUM_FINITE)
-                       return content2[i] == RANGE_DATUM_NEG_INF ? 1 : -1;
+                       if (content1[i] == content2[i])
+                               break;
+
+                       /* Otherwise, one bound is definitely larger than the other */
+                       if (content1[i] == RANGE_DATUM_NEG_INF)
+                               return -1;
+                       else if (content1[i] == RANGE_DATUM_POS_INF)
+                               return 1;
+                       else if (content2[i] == RANGE_DATUM_NEG_INF)
+                               return 1;
+                       else if (content2[i] == RANGE_DATUM_POS_INF)
+                               return -1;
+               }
 
                cmpval = DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[i],
                                                                                                 key->partcollation[i],