Hi Micheal, Em qua., 22 de mai. de 2024 às 21:21, Michael Paquier <mich...@paquier.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 >
