Ewan Young <[email protected]> 于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
