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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
