On Tue, Mar 29, 2022 at 11:58 AM Peter Geoghegan <p...@bowt.ie> wrote: > > I think I understand what the first paragraph of the header comment > > for heap_tuple_needs_freeze() is trying to say, but the second one is > > quite confusing. I think this is again because it veers into talking > > about what the caller should do rather than explaining what the > > function itself does. > > I wouldn't have done it that way if the function wasn't called > heap_tuple_needs_freeze(). > > I would be okay with removing this paragraph if the function was > renamed to reflect the fact it now tells the caller something about > the tuple having an old XID/MXID relative to the caller's own XID/MXID > cutoffs. Maybe the function name should be heap_tuple_would_freeze(), > making it clear that the function merely tells caller what > heap_prepare_freeze_tuple() *would* do, without presuming to tell the > vacuumlazy.c caller what it *should* do about any of the information > it is provided.
Attached is v13, which does it that way. This does seem like a real increase in clarity, albeit one that comes at the cost of renaming heap_tuple_needs_freeze(). v13 also addresses all of the other items from Robert's most recent round of feedback. I would like to commit something close to v13 on Friday or Saturday. Thanks -- Peter Geoghegan
v13-0003-vacuumlazy.c-Move-resource-allocation-to-heap_va.patch
Description: Binary data
v13-0002-Generalize-how-VACUUM-skips-all-frozen-pages.patch
Description: Binary data
v13-0001-Set-relfrozenxid-to-oldest-extant-XID-seen-by-VA.patch
Description: Binary data