On 09/04/2025 14:51, Tender Wang wrote:
> Hi,
>
> While working on another patch, I find that tablecmds.c now has three
> ways to check the validity of catalog tuples.
> i.
> if (tuple != NULL)
>
> ii.
> if (!tuple)
>
> iii.
> if (HeapTupleIsValid(tuple)
>
> In tablecmds.c, most checks use macro HeapTupleIsValid. For
> code readability,
> I changed the first and the second formats to the third one, e.g., using
> HeapTupleIsValid.
>
> BTW, I searched the other files, some files also have different ways to
> check the validity of tuples.
> But the attached patch only focuses on tablecmds.c
>
> Any thoughts?
It's a matter of taste, but personally I find 'if (tuple != NULL)' more
clear than 'if (HeapTupleIsValid(tuple))'. The presence of a macro
suggests that there might be other kinds of invalid tuples than a NULL
pointer, which just adds mental load.
Inconsistency is not good either though. I'm not sure it's worth the
churn, but I could get on board a patch to actually replace all
HeapTupleIsValid(tuple) calls with plain "tuple != NULL" checks. Keep
HeapTupleIsValid() just for compatibility, with a comment to discourage
using it.
--
Heikki Linnakangas
Neon (https://neon.tech)
