On Fri, Jan 12, 2024 at 2:32 PM Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Jan 12, 2024 at 1:52 PM Melanie Plageman > <melanieplage...@gmail.com> wrote: > This analysis seems correct to me, except that "when > lazy_scan_noprune() is called" should really say "when > lazy_scan_noprune() is called (and returns true)", because when it > returns false we fall through and call lazy_scan_prune() afterwards.
Now that I see your patch, I understand what Melanie must have meant. I agree that there is a small inconsistency here, that we could well do without. In general I am in favor of religiously eliminating such inconsistencies (between lazy_scan_prune and lazy_scan_noprune), unless there is a reason not to. Not because it's necessarily important. More because it's just too hard to be sure whether it might matter. It's usually easier to defensively assume that it matters. > Here's a draft patch to clean up the inconsistency here. It also gets > rid of recordfreespace, because ISTM that recordfreespace is adding to > the confusion here rather than helping anything. You're using "!prunestate.has_lpdead_items" as part of your test that sets "recordfreespace". But lazy_scan_noprune doesn't get passed a pointer to prunestate, so clearly you'll need to detect the same condition some other way. -- Peter Geoghegan
