On Tue, Dec 14, 2021 at 7:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ITYM "((actiondate::date))", but yeah, this leaks memory like there's
> no tomorrow. I traced it to 9dc718bdf (Pass down "logically unchanged
> index" hint), which has added a function index_unchanged_by_update()
> that (a) looks fairly expensive, (b) leaks a copy of every expression
> tree it examines, and (c) is invoked over again for each row, even
> though AFAICS the answer shouldn't change across rows. This seems very
> poorly thought through. Peter?
Ugh, what a howler. Clearly I am at fault here. Apologies.
Are you sure that it would really be worth the trouble of caching our
answer? It's not clear that that has only minimal maintenance burden.
I have always suspected that index_unchanged_by_update() was at least
slightly over-engineered.
The fact is that most individual aminsert() calls that get the hint
will never actually apply it in any way. In practice the hint is only
relevant when there isn't enough space on an nbtree leaf page to fit
the incoming item. Even then, it won't be used when there are LP_DEAD
bits set on the leaf page -- we prefer to perform a conventional index
deletion over a bottom-up index deletion. And so there is a fair
practical argument to be made in favor of assuming that we should give
the hint in cases where we can't rule it out inexpensively. Of course
that assumes that there will be no other use for the hint in the
future. I'm not making this argument myself, but it does seem like a
factor worth considering.
--
Peter Geoghegan