Обсуждение: Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

Поиск
Список
Период
Сортировка

Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

От
Álvaro Herrera
Дата:
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.)

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

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

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)



Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions

От
Andy Fan
Дата:
Á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