Re: Consistently use macro HeapTupleIsValid to check the validity of tuples in tablecmds.c

On 09.04.25 16:23, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
>> 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.
>
> Would you then advocate for also removing macros such as OidIsValid()
> and PointerIsValid()? That gets into a *lot* of code churn, and
> subsequent back-patching pain. We had a discussion about that
> just recently IIRC, and decided not to go there.

I'd generally be in favor of getting rid of these. Many of these
*IsValid macros generally don't actually do anything useful on top of
plain C code, and they add a level of mystery and confusion. No one is
adding new ones like that, so over time, the coding styles diverge. And
they distract from macros that actually do something useful like
AllocSizeIsValid().

In terms of backpatching effort/risk, here are some simple statistics:

git log --oneline REL_13_0..REL_13_20 | wc -l
1920

git log --oneline -G OidIsValid REL_13_0..REL_13_20 | wc -l
25

git log --oneline -G PointerIsValid REL_13_0..REL_13_20 | wc -l
5

So from that it would appear to be a relatively very small problem.