Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

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