Feedback-ID: ie0a040ee:Fastmail
Message-ID: <c40bf78d-9632-4442-9993-c71fbb3f6ced@eisentraut.org>
Date: Thu, 10 Apr 2025 14:25:24 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Consistently use macro HeapTupleIsValid to check the validity of
 tuples in tablecmds.c
To: Heikki Linnakangas <hlinnaka@iki.fi>, Tender Wang <tndrwang@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CAHewXNmz6huegXfMvBE=t-Ngyrh=sS=J2VB+hnhd+sadppdXww@mail.gmail.com>
 <9f5acdab-6519-4a3f-a67b-0e70fbc156e6@iki.fi>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <9f5acdab-6519-4a3f-a67b-0e70fbc156e6@iki.fi>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/c40bf78d-9632-4442-9993-c71fbb3f6ced%40eisentraut.org>
Precedence: bulk

On 09.04.25 14:26, Heikki Linnakangas wrote:
> 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.

agreed

> 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.

I'd be in favor of that.