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>, Michael Paquier <michael@paquier.xyz>, Nathan Bossart <nathandbossart@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2025-06-28T05:00:04Z
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

Attachments

Andy Fan <zhihuifan1213@163.com> writes:

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

The attached is the v2, I just change the commit message to address the
concern from Michael and Nathan. I also registed this into
https://commitfest.postgresql.org/patch/5870/.

Hi Michael and Nathan, has the new discussion addressed your concerns,
or do you still think we should keep them as it is?

Thanks
-- 
Best Regards
Andy Fan