Horton

Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

Andy Fan <zhihuifan1213@163.com>

View thread
From: Andy Fan <zhihuifan1213@163.com>
To: Álvaro Herrera <alvherre@kurilemu.de>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-06-27T00:02:17Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove HeapTupleheaderSetXminCommitted/Invalid functions

  2. Convert macros to static inline functions (htup_details.h, itup.h)

  3. Change the way we mark tuples as frozen.

  4. Code review for HeapTupleHeader changes. Add version number to page headers

  5. This patch wraps all accesses to t_xmin, t_cmin, t_xmax, and t_cmax in

Álvaro Herrera <alvherre@kurilemu.de> writes:

> Hmm
>
> So these functions were created from macros in commit 34694ec888d6,
> which themselves had been added for the first time in commit
> 37484ad2aace.  However, it appears that they were added only because
> they were mirroring HeapTupleHeaderSetXminFrozen(), and while the latter
> was immediately used, the other two weren't and never have been.
>
> (For a short while between June and September 2002 we had a different
> macro also called HeapTupleHeaderSetXminInvalid -- added by commit
> 3c35face4108 and removed by commit c7a165adc64e -- and curiously enough
> it was also entirely unused.)

Thanks for checking the history, and part of c7a165adc64e (removing
HeapTupleHeaderSetXminInvalid from htup.h) is pretty similar with the
case there.  

> I think Andy is right that these should be removed, not only because
> they are unsafe but because they are dead code.

Yes, I suggested with the two reasons. When I knew removing a public
API has potential to break some third-party extension even they are not
used in core, then the *unsafe* part encourage to do so.

Acutally no maintaince cost for the dead code is only true when no one
read/think about them, otherwise the cost is there. The
HeapTupleheaderSetXminCommitted has misleaded me to think I can do 
something there, but the fact is (1) they are never used. (2) the right 
place is SetHintBits. 

> codesearch.debian.net shows no matches by grep, other than
> htup_internals.h itself.

Thanks for sharing codesearch.decbian.net which looks a great project.

I just found even there is such code, core has already provided public
API HeapTupleSetHintBits which can be used as a replacement and it is
safe. 

-- 
Best Regards
Andy Fan