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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Avoid possible dereference null pointer (src/backend/catalog/pg_depend.c)
Date: 2024-05-23 00:21:32
Message-ID: Zk6MDC1P9UinxTS-@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

> 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.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2024-05-23 00:27:41 Re: Shared detoast Datum proposal
Previous Message Noah Misch 2024-05-23 00:05:48 Inval reliability, especially for inplace updates