On Fri, Apr 5, 2019 at 9:05 AM Andres Freund <andres@anarazel.de> wrote:
I think the right approach would be to do all of this in heap_insert and heap_multi_insert. Whenever starting to work on a page, if INSERT_FROZEN is specified, remember whether it is either currently empty, or is already marked as all-visible. If previously empty, mark it as all visible at the end. If already all visible, there's no need to change that. If not yet all-visible, no need to do anything, since it can't have been inserted with COPY FREEZE.
We're doing roughly the same. If we are running INSERT_FROZEN, whenever we're about to switch to a new page, we check if the previous page should be marked all-frozen and do it that way. The special code in copy.c is necessary to take care of the last page which we don't get to handle in the regular code path.
Or are you suggesting that we don't even rescan the page for all-frozen tuples at the end and just simply mark it all-frozen at the start, when the first tuple is inserted and then don't touch the PD_ALL_VISIBLE/visibility map bit as we go on inserting more tuples in the page?
Anyways, if major architectural changes are required then it's probably too late to consider this for PG12, even though it's more of a bug fix and a candidate for back-patching too.
