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)
