public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Tender Wang <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Re: Consistently use macro HeapTupleIsValid to check the validity of tuples in tablecmds.c
Date: Wed, 9 Apr 2025 15:26:07 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHewXNmz6huegXfMvBE=t-Ngyrh=sS=J2VB+hnhd+sadppdXww@mail.gmail.com>
References: <CAHewXNmz6huegXfMvBE=t-Ngyrh=sS=J2VB+hnhd+sadppdXww@mail.gmail.com>

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)

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Consistently use macro HeapTupleIsValid to check the validity of tuples in tablecmds.c
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox