Hi, On 2019-04-04 23:57:58 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > 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. Do you see any problem doing it > > that way? > > Do we want to add overhead to these hot-spot routines for this purpose?
For heap_multi_insert I can't see it being a problem - it's only used from copy.c, and the cost should be "smeared" over many tuples. I'd assume that compared with locking a page, WAL logging, etc, it'd not even meaningfully show up for heap_insert. Especially because we already have codepaths for options & HEAP_INSERT_FROZEN in heap_prepare_insert(), and I'd assume those could be combined. I think we should measure it, but I don't think that one or two additional, very well predictd, branches are going to be measurable in in those routines. The patch, as implemented, has modifications in RelationGetBufferForTuple(), that seems like they'd be more expensive. Greetings, Andres Freund
