Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4025e6c

Browse files
committed
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
1 parent ad84ecc commit 4025e6c

File tree

2 files changed

+9
-14
lines changed

2 files changed

+9
-14
lines changed

src/backend/partitioning/partbounds.c

+6-7
Original file line numberDiff line numberDiff line change
@@ -4684,6 +4684,8 @@ compute_partition_hash_value(int partnatts, FmgrInfo *partsupfunc, Oid *partcoll
46844684
*
46854685
* Returns true if remainder produced when this computed single hash value is
46864686
* divided by the given modulus is equal to given remainder, otherwise false.
4687+
* NB: it's important that this never return null, as the constraint machinery
4688+
* would consider that to be a "pass".
46874689
*
46884690
* See get_qual_for_hash() for usage.
46894691
*/
@@ -4708,9 +4710,9 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
47084710
ColumnsHashData *my_extra;
47094711
uint64 rowHash = 0;
47104712

4711-
/* Return null if the parent OID, modulus, or remainder is NULL. */
4713+
/* Return false if the parent OID, modulus, or remainder is NULL. */
47124714
if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2))
4713-
PG_RETURN_NULL();
4715+
PG_RETURN_BOOL(false);
47144716
parentId = PG_GETARG_OID(0);
47154717
modulus = PG_GETARG_INT32(1);
47164718
remainder = PG_GETARG_INT32(2);
@@ -4740,14 +4742,11 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
47404742
int j;
47414743

47424744
/* Open parent relation and fetch partition key info */
4743-
parent = try_relation_open(parentId, AccessShareLock);
4744-
if (parent == NULL)
4745-
PG_RETURN_NULL();
4745+
parent = relation_open(parentId, AccessShareLock);
47464746
key = RelationGetPartitionKey(parent);
47474747

47484748
/* Reject parent table that is not hash-partitioned. */
4749-
if (parent->rd_rel->relkind != RELKIND_PARTITIONED_TABLE ||
4750-
key->strategy != PARTITION_STRATEGY_HASH)
4749+
if (key == NULL || key->strategy != PARTITION_STRATEGY_HASH)
47514750
ereport(ERROR,
47524751
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
47534752
errmsg("\"%s\" is not a hash partitioned table",

src/test/regress/expected/hash_part.out

+3-7
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ CREATE TABLE mchash1
1010
PARTITION OF mchash FOR VALUES WITH (MODULUS 4, REMAINDER 0);
1111
-- invalid OID, no such table
1212
SELECT satisfies_hash_partition(0, 4, 0, NULL);
13-
satisfies_hash_partition
14-
--------------------------
15-
16-
(1 row)
17-
13+
ERROR: could not open relation with OID 0
1814
-- not partitioned
1915
SELECT satisfies_hash_partition('tenk1'::regclass, 4, 0, NULL);
2016
ERROR: "tenk1" is not a hash partitioned table
@@ -34,14 +30,14 @@ ERROR: remainder for hash partition must be less than modulus
3430
SELECT satisfies_hash_partition('mchash'::regclass, NULL, 0, NULL);
3531
satisfies_hash_partition
3632
--------------------------
37-
33+
f
3834
(1 row)
3935

4036
-- remainder is null
4137
SELECT satisfies_hash_partition('mchash'::regclass, 4, NULL, NULL);
4238
satisfies_hash_partition
4339
--------------------------
44-
40+
f
4541
(1 row)
4642

4743
-- too many arguments

0 commit comments

Comments
 (0)