Re: Avoid possible dereference null pointer (src/backend/catalog/pg_depend.c)

Hi Micheal,

Em qua., 22 de mai. de 2024 às 21:21, Michael Paquier <michael(at)paquier(dot)xyz>
escreveu:

> On Wed, May 22, 2024 at 03:28:48PM -0300, Ranier Vilela wrote:
> > 1. Another concern is the function *get_partition_ancestors*,
> > which may return NIL, which may affect *llast_oid*, which does not handle
> > NIL entries.
>
> Hm? We already know in the code path that the relation we are dealing
> with when calling get_partition_ancestors() *is* a partition thanks to
> the check on relispartition, no? In this case, calling
> get_partition_ancestors() is valid and there should be a top-most
> parent in any case all the time. So I don't get the point of checking
> get_partition_ancestors() for NIL-ness just for the sake of assuming
> that it would be possible.
>
I don't have strong feelings about this.
But analyzing the function, *pg_partition_root*
(src/backend/utils/adt/partitionfuncs.c),
we see that checking whether it is a partition is done by
check_rel_can_be_partition.
And it doesn't trust get_partition_ancestors, checking
if the return is NIL.

>
> > 2. Is checking *relispartition* enough?
> > There a function *check_rel_can_be_partition*
> > (src/backend/utils/adt/partitionfuncs.c),
> > which performs a much more robust check, would it be worth using it?
> >
> > With the v2 attached, 1 is handled, but, in this case,
> > will it be the most correct?
>
> Saying that, your point about the result of SearchSysCacheAttName not
> checked if it is a valid tuple is right. We paint errors in these
> cases even if they should not happen as that's useful when it comes to
> debugging, at least.
>
Thanks.

best regards,
Ranier Vilela

> --
> Michael
>