Do not return NULL for error cases in satisfies_hash_partition().

Since this function is used as a CHECK constraint condition,
returning NULL is tantamount to returning TRUE, which would have the
effect of letting in a row that doesn't satisfy the hash condition.
Admittedly, the cases for which this is done should be unreachable
in practice, but that doesn't make it any less a bad idea.  It also
seems like a dartboard was used to decide which error cases should
throw errors as opposed to returning NULL.

For the checks for NULL input values, I just switched it to returning
false.  There's some argument that an error would be better; but the
case really should be can't-happen in a generated hash constraint,
so it's likely not worth more code for.

For the parent-relation-open-failure case, it seems like we might
as well let relation_open throw an error, instead of having an
impossible-to-diagnose constraint failure.

Back-patch to v11 where this code came in.

Discussion: https://postgr.es/m/24067.1605134819@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2020-11-16 16:39:59 -05:00
parent 029fa664ec
commit 54a9406440
2 changed files with 10 additions and 15 deletions

View File

@ -2778,6 +2778,8 @@ compute_partition_hash_value(int partnatts, FmgrInfo *partsupfunc, Oid *partcoll
* *
* Returns true if remainder produced when this computed single hash value is * Returns true if remainder produced when this computed single hash value is
* divided by the given modulus is equal to given remainder, otherwise false. * divided by the given modulus is equal to given remainder, otherwise false.
* NB: it's important that this never return null, as the constraint machinery
* would consider that to be a "pass".
* *
* See get_qual_for_hash() for usage. * See get_qual_for_hash() for usage.
*/ */
@ -2802,9 +2804,9 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
ColumnsHashData *my_extra; ColumnsHashData *my_extra;
uint64 rowHash = 0; uint64 rowHash = 0;
/* Return null if the parent OID, modulus, or remainder is NULL. */ /* Return false if the parent OID, modulus, or remainder is NULL. */
if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2)) if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2))
PG_RETURN_NULL(); PG_RETURN_BOOL(false);
parentId = PG_GETARG_OID(0); parentId = PG_GETARG_OID(0);
modulus = PG_GETARG_INT32(1); modulus = PG_GETARG_INT32(1);
remainder = PG_GETARG_INT32(2); remainder = PG_GETARG_INT32(2);
@ -2833,15 +2835,12 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
PartitionKey key; PartitionKey key;
int j; int j;
/* Open parent relation and fetch partition keyinfo */ /* Open parent relation and fetch partition key info */
parent = try_relation_open(parentId, AccessShareLock); parent = relation_open(parentId, AccessShareLock);
if (parent == NULL)
PG_RETURN_NULL();
key = RelationGetPartitionKey(parent); key = RelationGetPartitionKey(parent);
/* Reject parent table that is not hash-partitioned. */ /* Reject parent table that is not hash-partitioned. */
if (parent->rd_rel->relkind != RELKIND_PARTITIONED_TABLE || if (key == NULL || key->strategy != PARTITION_STRATEGY_HASH)
key->strategy != PARTITION_STRATEGY_HASH)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE), (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("\"%s\" is not a hash partitioned table", errmsg("\"%s\" is not a hash partitioned table",

View File

@ -10,11 +10,7 @@ CREATE TABLE mchash1
PARTITION OF mchash FOR VALUES WITH (MODULUS 4, REMAINDER 0); PARTITION OF mchash FOR VALUES WITH (MODULUS 4, REMAINDER 0);
-- invalid OID, no such table -- invalid OID, no such table
SELECT satisfies_hash_partition(0, 4, 0, NULL); SELECT satisfies_hash_partition(0, 4, 0, NULL);
satisfies_hash_partition ERROR: could not open relation with OID 0
--------------------------
(1 row)
-- not partitioned -- not partitioned
SELECT satisfies_hash_partition('tenk1'::regclass, 4, 0, NULL); SELECT satisfies_hash_partition('tenk1'::regclass, 4, 0, NULL);
ERROR: "tenk1" is not a hash partitioned table ERROR: "tenk1" is not a hash partitioned table
@ -34,14 +30,14 @@ ERROR: remainder for hash partition must be less than modulus
SELECT satisfies_hash_partition('mchash'::regclass, NULL, 0, NULL); SELECT satisfies_hash_partition('mchash'::regclass, NULL, 0, NULL);
satisfies_hash_partition satisfies_hash_partition
-------------------------- --------------------------
f
(1 row) (1 row)
-- remainder is null -- remainder is null
SELECT satisfies_hash_partition('mchash'::regclass, 4, NULL, NULL); SELECT satisfies_hash_partition('mchash'::regclass, 4, NULL, NULL);
satisfies_hash_partition satisfies_hash_partition
-------------------------- --------------------------
f
(1 row) (1 row)
-- too many arguments -- too many arguments