Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

Michael Paquier <michael@paquier.xyz> writes:

> On Wed, Jun 25, 2025 at 07:47:27AM +0000, Andy Fan wrote:
>> When I am reading the code, I first thought I can do something in
>> HeapTupleheaderSetXminCommitted, then I realized we have SetHintBits.
>> After some research, I find HeapTupleHeaderSetXminCommitted is never
>> used and it looks not safe to use after comparing with SetHintBits. So
>> to avoid future confusion or misuse, I'd suggest to remove it. I think
>> HeapTupleHeaderSetXminInvalid should be the same. So here is the patch.
>
> There is a cost in removing such code, even if not used in core:
> extension code outside of core may use it, and they would fail to
> compile.  This can break code, and keeping them around has no
> maintenance cost.

I did thought about the impact on third-party extension, but I suggested
the patch not only because it is not used but also it is not safe to set
the flags directly. What do you think about this?

From comments of SetHintBits:

 * It is only safe to set a transaction-committed hint bit if we know the
 * transaction's commit record is guaranteed to be flushed to disk before the
 * buffer, or if the table is temporary or unlogged and will be obliterated by
 * a crash anyway.  We cannot change the LSN of the page here, because we may

Just in case someone use it, it may ask for public API for SetHintBits. 

Thanks for have a look.

-- 
Best Regards
Andy Fan