Re: Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]

Hey Alvaro, can you comment on this bit in the proposed patch?

+        for (i = FirstOffsetNumber; i <= itemcount; i++)
+        {
+            ItemId        ii = PageGetItemId(phdr, i);
+
+            /* Normally here was following assertion
+             * Assert(ItemIdHasStorage(ii));
+             * This assertion was introduced in PageIndexTupleDelete
+             * But if this function will be used from BRIN index
+             * this assertion will fail. Thus, here we do not check that
+             * item has the storage.
+             */
+            if (ItemIdGetOffset(ii) <= offset)
+                ii->lp_off += size_diff;
+        }
+    }

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?
        regards, tom lane