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