Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access) - Mailing list pgsql-hackers

On Wed, Jan 7, 2026 at 3:15 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> I believe the reason why we add Assert(TransactionIdIsValid(dead_after)) under HEAPTUPLE_RECENTLY_DEAD is to ensure
thatwhen HeapTupleSatisfiesVacuumHorizon() returns HEAPTUPLE_RECENTLY_DEAD, dead_after must be set. So the goal of the
assertis to catch bugs of HeapTupleSatisfiesVacuumHorizon(). 
>
> From this perspective, I now feel dead_after should be initialized to InvalidTransactionId. Otherwise, say
HeapTupleSatisfiesVacuumHorizon()has a bug and miss to set dead_after, then the assert mostly like won’t be fired,
becauseit holds a random value, most likely not be 0. 

Actually, thinking about it more, I decided to remove the assertions
on dead_after from those patches entirely. I don't use dead_after and
only pass it in because HeapTupleSatisfiesVacuumHorizon requires it.
In fact, I don't care if the function correctly sets dead_after since
I don't use it.

> +       /* set if the query doesn't modify the rel */
> +       SO_HINT_REL_READ_ONLY = 1 << 10,
> ```
>
> Nit: I think it’s better to replace “rel” to “relation”. For a function comment, if there is a parameter named “rel”,
thenwe can use it to refer to the parameter, without such a context, I guess here a while word is better. 

k

I'm currently working on a new version that incorporates Andres'
review feedback and will post soon.

- Melanie