Re: satisfies_hash_partition crash

Ewan Young <kdbase(dot)hack(at)gmail(dot)com> 于2026年7月2日周四 13:43写道：
>
> Thanks for taking a look, and for confirming the non-variadic point.
>
> I think that placement doesn't quite cover it, though. There are two
> PG_GETARG_ARRAYTYPE_P(3) calls:
> the one in the cache-setup branch you patched (reached only on the
> first call, or when the parent OID changes),
> and a second one in the deconstruct_array() path that runs on every
> call. Because fn_extra is cached across calls,
> a NULL array can skip the cache-setup block and reach that second
> fetch unguarded — so it still crashes:
>
> create table hp (a int) partition by hash (a);
> create table hp0 partition of hp for values with (modulus 2, remainder 0);
> create table hp1 partition of hp for values with (modulus 2, remainder 1);
> create table t (arr int[]);
> insert into t values (array[1]), (null), (array[2]);
> select satisfies_hash_partition('hp'::regclass, 2, 0, variadic arr) from t;
>
> The first (non-NULL) row builds the cache, and the NULL row then
> crashes in the deconstruct path.
> With the top-level check it returns t/f/t. (The cache-setup placement
> also returns between relation_open()
> and relation_close(), so it trips "resource was not closed" even for
> the single-NULL case.)

Yes. The previous fix cannot prevent a crash if my_extra is not NULL,
as your case shows
And I forgot to call relation_close(), so "resource was not closed" would occur.
Anyway, the NULL-array guard must sit ahead.

>
> You're right that the top-level version calls get_fn_expr_variadic()
> once more in the non-variadic path.
> It's just an O(1) read of the funcvariadic flag, so I left it as is —
> but if you'd rather not repeat it, we could
> compute it once into a local near the top and reuse it. Either way the
> NULL-array guard needs to sit ahead
> of both array fetches.

Personally, I prefer a local bool variable near the top and reuse it.

--
Thanks,
Tender Wang