Fix bogus logic for reporting which hash partition conflicts.

Commit efbfb64 added logic for reporting exactly which existing
partition conflicts when complaining that a new hash partition's
modulus isn't compatible with the existing ones.  However, it
misunderstood the partitioning data structure, and would select
the wrong partition in some cases, or crash outright due to fetching
a bogus table OID in other cases.

Per bug #17076 from Alexander Lakhin.  Fix by Amit Langote;
some further work on the code comments by me.

Discussion: https://postgr.es/m/17076-89a16ae835d329b9@postgresql.org

Author: tglsfdc

src/backend/partitioning/partbounds.c

@@ -2838,12 +2838,15 @@ check_new_partition_bound(char *relname, Relation parent,
 
 					/*
 					 * Check rule that every modulus must be a factor of the
-					 * next larger modulus.  For example, if you have a bunch
+					 * next larger modulus.  (For example, if you have a bunch
 					 * of partitions that all have modulus 5, you can add a
 					 * new partition with modulus 10 or a new partition with
 					 * modulus 15, but you cannot add both a partition with
 					 * modulus 10 and a partition with modulus 15, because 10
-					 * is not a factor of 15.
+					 * is not a factor of 15.)  We need only check the next
+					 * smaller and next larger existing moduli, relying on
+					 * previous enforcement of this rule to be sure that the
+					 * rest are in line.
 					 */
 
 					/*
@@ -2870,15 +2873,16 @@ check_new_partition_bound(char *relname, Relation parent,
 									 errmsg("every hash partition modulus must be a factor of the next larger modulus"),
 									 errdetail("The new modulus %d is not a factor of %d, the modulus of existing partition \"%s\".",
 											   spec->modulus, next_modulus,
-											   get_rel_name(partdesc->oids[boundinfo->indexes[0]]))));
+											   get_rel_name(partdesc->oids[0]))));
 					}
 					else
 					{
 						int			prev_modulus;
 
 						/*
-						 * We found the largest modulus less than or equal to
-						 * ours.
+						 * We found the largest (modulus, remainder) pair less
+						 * than or equal to the new one.  That modulus must be
+						 * a divisor of, or equal to, the new modulus.
 						 */
 						prev_modulus = DatumGetInt32(boundinfo->datums[offset][0]);
 
@@ -2889,13 +2893,19 @@ check_new_partition_bound(char *relname, Relation parent,
 									 errdetail("The new modulus %d is not divisible by %d, the modulus of existing partition \"%s\".",
 											   spec->modulus,
 											   prev_modulus,
-											   get_rel_name(partdesc->oids[boundinfo->indexes[offset]]))));
+											   get_rel_name(partdesc->oids[offset]))));
 
 						if (offset + 1 < boundinfo->ndatums)
 						{
 							int			next_modulus;
 
-							/* Look at the next higher modulus */
+							/*
+							 * Look at the next higher (modulus, remainder)
+							 * pair.  That could have the same modulus and a
+							 * larger remainder than the new pair, in which
+							 * case we're good.  If it has a larger modulus,
+							 * the new modulus must divide that one.
+							 */
 							next_modulus = DatumGetInt32(boundinfo->datums[offset + 1][0]);
 
 							if (next_modulus % spec->modulus != 0)
@@ -2904,7 +2914,7 @@ check_new_partition_bound(char *relname, Relation parent,
 										 errmsg("every hash partition modulus must be a factor of the next larger modulus"),
 										 errdetail("The new modulus %d is not a factor of %d, the modulus of existing partition \"%s\".",
 												   spec->modulus, next_modulus,
-												   get_rel_name(partdesc->oids[boundinfo->indexes[offset + 1]]))));
+												   get_rel_name(partdesc->oids[offset + 1]))));
 						}
 					}
 

src/test/regress/expected/create_table.out

@@ -780,14 +780,20 @@ CREATE TABLE hash_parted (
 CREATE TABLE hpart_1 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 0);
 CREATE TABLE hpart_2 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 50, REMAINDER 1);
 CREATE TABLE hpart_3 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 200, REMAINDER 2);
+CREATE TABLE hpart_4 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 3);
 -- modulus 25 is factor of modulus of 50 but 10 is not a factor of 25.
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 25, REMAINDER 3);
 ERROR:  every hash partition modulus must be a factor of the next larger modulus
-DETAIL:  The new modulus 25 is not divisible by 10, the modulus of existing partition "hpart_1".
+DETAIL:  The new modulus 25 is not divisible by 10, the modulus of existing partition "hpart_4".
 -- previous modulus 50 is factor of 150 but this modulus is not a factor of next modulus 200.
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 150, REMAINDER 3);
 ERROR:  every hash partition modulus must be a factor of the next larger modulus
 DETAIL:  The new modulus 150 is not a factor of 200, the modulus of existing partition "hpart_3".
+-- overlapping remainders
+CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 100, REMAINDER 3);
+ERROR:  partition "fail_part" would overlap partition "hpart_4"
+LINE 1: ...BLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODU...
+                                                             ^
 -- trying to specify range for the hash partitioned table
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES FROM ('a', 1) TO ('z');
 ERROR:  invalid bound specification for a hash partition
@@ -1100,7 +1106,7 @@ Number of partitions: 3 (Use \d+ to list them.)
 --------+---------+-----------+----------+---------
  a      | integer |           |          | 
 Partition key: HASH (a)
-Number of partitions: 3 (Use \d+ to list them.)
+Number of partitions: 4 (Use \d+ to list them.)
 
 -- check that we get the expected partition constraints
 CREATE TABLE range_parted4 (a int, b int, c int) PARTITION BY RANGE (abs(a), abs(b), c);

src/test/regress/sql/create_table.sql

@@ -631,10 +631,13 @@ CREATE TABLE hash_parted (
 CREATE TABLE hpart_1 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 0);
 CREATE TABLE hpart_2 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 50, REMAINDER 1);
 CREATE TABLE hpart_3 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 200, REMAINDER 2);
+CREATE TABLE hpart_4 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 10, REMAINDER 3);
 -- modulus 25 is factor of modulus of 50 but 10 is not a factor of 25.
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 25, REMAINDER 3);
 -- previous modulus 50 is factor of 150 but this modulus is not a factor of next modulus 200.
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 150, REMAINDER 3);
+-- overlapping remainders
+CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES WITH (MODULUS 100, REMAINDER 3);
 -- trying to specify range for the hash partitioned table
 CREATE TABLE fail_part PARTITION OF hash_parted FOR VALUES FROM ('a', 1) TO ('z');
 -- trying to specify list value for the hash partitioned table