[Peter Eisentraut <peter.eisentraut@enterprisedb.com>]

I had a bit of trouble parsing the error message "every hash partition 
modulus must be a factor of the next larger modulus", so I went into the 
code, added some comments and added errdetail messages for each case.  I 
think it's a bit clearer now.

Attachments:

0001-Improve-new-hash-partition-bound-check-error-message.patch (text/plain, 6.1K) download
From e7f392e2f8950236a22f007cc3aed36729da22e1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 2 Feb 2021 11:21:21 +0100
Subject: [PATCH] Improve new hash partition bound check error messages

For the error message "every hash partition modulus must be a factor
of the next larger modulus", add a detail message that shows the
particular numbers involved.  Also comment the code more.
---
 src/backend/partitioning/partbounds.c      | 58 +++++++++++++++-------
 src/test/regress/expected/alter_table.out  |  1 +
 src/test/regress/expected/create_table.out |  2 +
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 0c3f212ff2..7425e34040 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -2832,14 +2832,9 @@ check_new_partition_bound(char *relname, Relation parent,
 
 				if (partdesc->nparts > 0)
 				{
-					Datum	  **datums = boundinfo->datums;
-					int			ndatums = boundinfo->ndatums;
 					int			greatest_modulus;
 					int			remainder;
 					int			offset;
-					bool		valid_modulus = true;
-					int			prev_modulus,	/* Previous largest modulus */
-								next_modulus;	/* Next largest modulus */
 
 					/*
 					 * Check rule that every modulus must be a factor of the
@@ -2849,7 +2844,9 @@ check_new_partition_bound(char *relname, Relation parent,
 					 * 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.
-					 *
+					 */
+
+					/*
 					 * Get the greatest (modulus, remainder) pair contained in
 					 * boundinfo->datums that is less than or equal to the
 					 * (spec->modulus, spec->remainder) pair.
@@ -2859,26 +2856,51 @@ check_new_partition_bound(char *relname, Relation parent,
 													spec->remainder);
 					if (offset < 0)
 					{
-						next_modulus = DatumGetInt32(datums[0][0]);
-						valid_modulus = (next_modulus % spec->modulus) == 0;
+						int			next_modulus;
+
+						/*
+						 * All existing moduli are greater or equal, so the
+						 * new one must be a factor of the smallest one, which
+						 * is first in the boundinfo.
+						 */
+						next_modulus = DatumGetInt32(boundinfo->datums[0][0]);
+						if (next_modulus % spec->modulus != 0)
+							ereport(ERROR,
+									(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+									 errmsg("every hash partition modulus must be a factor of the next larger modulus"),
+									 errdetail("The new modulus %d is not a factor of the existing modulus %d.",
+											   spec->modulus, next_modulus)));
 					}
 					else
 					{
-						prev_modulus = DatumGetInt32(datums[offset][0]);
-						valid_modulus = (spec->modulus % prev_modulus) == 0;
+						int			prev_modulus;
 
-						if (valid_modulus && (offset + 1) < ndatums)
+						/* We found the largest modulus less than or equal to ours. */
+						prev_modulus = DatumGetInt32(boundinfo->datums[offset][0]);
+
+						if (spec->modulus % prev_modulus != 0)
+							ereport(ERROR,
+									(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+									 errmsg("every hash partition modulus must be a factor of the next larger modulus"),
+									 errdetail("The existing modulus %d is not a factor of the new modulus %d.",
+											   prev_modulus, spec->modulus)));
+
+						if (offset + 1 < boundinfo->ndatums)
 						{
-							next_modulus = DatumGetInt32(datums[offset + 1][0]);
-							valid_modulus = (next_modulus % spec->modulus) == 0;
+							int			next_modulus;
+
+							/* Look at the next higher modulus */
+							next_modulus = DatumGetInt32(boundinfo->datums[offset + 1][0]);
+
+							if (next_modulus % spec->modulus != 0)
+								ereport(ERROR,
+										(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+										 errmsg("every hash partition modulus must be a factor of the next larger modulus"),
+										 errdetail("The new modulus %d is not factor of the existing modulus %d.",
+												   spec->modulus, next_modulus)));
 						}
 					}
 
-					if (!valid_modulus)
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-								 errmsg("every hash partition modulus must be a factor of the next larger modulus")));
-
 					greatest_modulus = boundinfo->nindexes;
 					remainder = spec->remainder;
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0ce6ee4622..62bf27fc50 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4109,6 +4109,7 @@ ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 8, R
 ERROR:  remainder for hash partition must be less than modulus
 ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, REMAINDER 2);
 ERROR:  every hash partition modulus must be a factor of the next larger modulus
+DETAIL:  The new modulus 3 is not a factor of the existing modulus 4.
 DROP TABLE fail_part;
 --
 -- DETACH PARTITION
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index ed8c01b8de..95d8ed4c45 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -783,9 +783,11 @@ CREATE TABLE hpart_3 PARTITION OF hash_parted FOR VALUES WITH (MODULUS 200, REMA
 -- modulus 25 is factor of modulus of 50 but 10 is not 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 existing modulus 10 is not a factor of the new modulus 25.
 -- previous modulus 50 is factor of 150 but this modulus is not 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 factor of the existing modulus 200.
 -- 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
-- 
2.30.0