On 09.04.25 16:23, Tom Lane wrote:
Heikki Linnakangas <hlinn...@iki.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.
