Alvaro Herrera <alvhe...@2ndquadrant.com> writes: > Tom Lane wrote: >> That comment seems utterly wrong to me, because both PageIndexTupleDelete >> and PageIndexMultiDelete certainly contain assertions that every item on >> the page has storage. Are you expecting that any BRIN index items >> wouldn't? If they wouldn't, is adjusting their lp_off as if they did >> have storage sensible?
> It is possible in BRIN to have empty intermediate tuples; for example it > is possible for lp 1 and 3 to contain index tuples, while lp 2 does not. Hm. So apparently, the only reason this stuff works at all is that BRIN isn't using either PageIndexTupleDelete or PageIndexMultiDelete. > Now if this loop is concerned only with live lps and does not move lps, > then it should be fine to add the assertion. No, it iterates over all lps on the page. I'm inclined to think it should be written like if (ItemIdHasStorage(ii) && ItemIdGetOffset(ii) <= offset) ii->lp_off += size_diff; because futzing with the lp_off field in an item that isn't really pointing at storage feels wrong. We might need to do that to PageIndexTupleDelete and/or PageIndexMultiDelete someday, too. I notice that PageIndexDeleteNoCompact, which seems to express what BRIN is expecting in a rather underdocumented way, forces any items without storage into "unused" state. I don't really think it's bufpage.c's place to do that, though. Do you think that code is actually supposed to fire, or is it just there for lack of a better idea? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers