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

On Tue, Jan 6, 2026 at 4:40 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
>
> > <v32-0014-Pass-down-information-on-table-modification-to-s.patch>
>
> I've tried to take an attempt to review some patches of this patchset. It's huge and mostly polished.

I've added attributed your review on the patches you specifically
mention here (and from previous emails you sent). Let me know if there
are other patches you reviewed that you did not mention.

> In a step "Pass down information on table modification to scan node" you pass SO_HINT_REL_READ_ONLY flag in
IndexNext()and BitmapTableScanSetup(), but not in IndexNextWithReorder() and IndexOnlyNext(). Is there a reason why
indexscans with ordering cannot use on-access VM setting? 

Great point, I simply hadn't tested those cases and didn't think to
add them. I've added them in attached v33.

While looking at other callers of index_beginscan(), I was wondering
if systable_beginscan() and systable_beginscan_ordered() should ever
pass SO_HINT_REL_READ_ONLY. I guess we would need to pass if the
operation is read-only above the index_beginscan() -- I'm not sure if
we always know in the caller of systable_beginscan() whether this
operation will modify the catalog. That seems like it could be a
separate project, though, so maybe it is better to say this feature is
just for regular tables.

As for the other cases: We don't have the relation range table index
in check_exclusion_or_unique_constraints(), so I don't think we can do
it there.

And I think that the other index scan cases like in replication code
or get_actual_variable_endpoint() are too small to be worth it, don't
have the needed info, or don't do on-access pruning (bc of the
snapshot type they use).

> Also, comment about visibilitymap_set() says "Callers that log VM changes separately should use visibilitymap_set()"
asif visibilitymap_set() is some other function. 

Ah, yes, I forgot to remove that when I removed the old
visibilitymap_set() and made visibilitymap_set_vmbits() into
visiblitymap_set(). Done in v33.

- Melanie