On Tue, 9 Mar 2021 at 22:35, Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> Matthias van de Meent <boekewurm+postg...@gmail.com> writes:
> > The only two existing mechanisms that I could find (in the access/heap
> > directory) that possibly could fail on shrunken line pointer arrays;
> > being xlog recovery (I do not have enough knowledge on recovery to
> > determine if that may touch pages that have shrunken line pointer
> > arrays, or if those situations won't exist due to never using dirtied
> > pages in recovery) and backwards table scans on non-MVCC snapshots
> > (which would be fixed in the attached patch).
>
> I think you're not visualizing the problem properly.
>
> The case I was concerned about back when is that there are various bits of
> code that may visit a page with a predetermined TID in mind to look at.
> An index lookup is an obvious example, and another one is chasing an
> update chain's t_ctid link. You might argue that if the tuple was dead
> enough to be removed, there should be no such in-flight references to
> worry about, but I think such an assumption is unsafe. There is not, for
> example, any interlock that ensures that a process that has obtained a TID
> from an index will have completed its heap visit before a VACUUM that
> subsequently removed that index entry also removes the heap tuple.
I am aware of this problem. I will admit that I did not detected all
potentially problematic accesses, so I'll show you my work.
> So, to accept a patch that shortens the line pointer array, what we need
> to do is verify that every such code path checks for an out-of-range
> offset before trying to fetch the target line pointer. I believed
> back in 2007 that there were, or once had been, code paths that omitted
> such a range check, assuming that they could trust the TID they had
> gotten from $wherever to point at an extant line pointer array entry.
> Maybe things are all good now, but I think you should run around and
> examine every place that checks for tuple deadness to see if the offset
> it used is known to be within the current page bounds.
In my search for problematic accesses, I make the following assumptions:
* PageRepairFragmentation as found in bufpage is only applicable to
heap pages; this function is not applied to other pages in core
postgres. So, any problems that occur are with due to access with an
OffsetNumber > PageGetMaxOffsetNumber.
* Items [on heap pages] are only extracted after using PageGetItemId
for that item on the page, whilst holding a lock.
Under those assumptions, I ran "grep PageGetItemId" over the src
directory. For all 227 results (as of 68b34b23) I checked if the page
accessed (or item accessed thereafter) was a heap page or heap tuple.
After analysis of the relevant references, I had the following results
(attached full report, containing a line with the file & line number,
and code line of the call, followed by a line containing the usage
type):
Count of usage type - usage type
4 - Not a call (comment)
7 - Callsite guarantees bounds
8 - Has assertion ItemIdIsNormal (asserts item is not removed; i.e.
concurrent vacuum should not have been able to remove this item)
39 - Has bound checks
6 - Not a heap page (brin)
1 - Not a heap page (generic index)
24 - Not a heap page (gin)
21 - Not a heap page (gist)
14 - Not a heap page (hash)
60 - Not a heap page (nbtree)
1 - Not a heap page (sequence)
36 - Not a heap page (spgist)
2 - OffsetNumber is generated by PageAddItem
2 - problem case 1 (table scan)
1 - Problem case 2 (xlog)
1 - Problem case 3 (invalid redirect pointers)
The 3 problem cases were classified based on the origin of the
potentially invalid pointer.
Problem case 1: table scan; heapam.c lines 678 and 811, in heapgettup
The table scan maintains a state which contains a page-bound
OffsetNumber, which it uses as a cursor whilst working through the
pages of the relation. In forward scans, the bounds of the page are
re-validated at the start of the 'while (linesleft > 0)' loop at 681,
but for backwards scans this check is invalid, because it incorrectly
assumes that the last OffsetNumber is guaranteed to still exist.
For MVCC snapshots, this is true (the previously returned value must
have been visible in its snapshot, therefore cannot have been vacuumed
because the snapshot is still alive), but non-mvcc snapshots may have
returned a dead tuple, which is now vacuumed and truncated away.
The first occurrance of this issue is easily fixed with the changes as
submitted in patch v2.
The second problem case (on line 811) is for forward scans, where the
line pointer array could have been truncated to 0 length. As the code
uses a hardcoded offset of FirstOffsetNumber (=1), that might point
into arbitrary data. The reading of this arbitrary data is saved by
the 'while (linesleft > 0) check', because at that point linesleft
will be PageGetMaxOffsetNumber, which would then equal 0.
Problem case 2: xlog; heapam.c line 8796, in heap_xlog_freeze_page
This is in the replay of transaction logs, in heap_xlog_freeze_page.
As I am unaware whether or not pages to which these transaction logs
are applied can contain changes from the xlog-generating instance, I
flagged this as a potential problem. The application of the xlogs is
probably safe (it assumes the existence of a HeapTupleHeader for that
ItemId), but my limited knowledge put this on the 'potential problem'
list.
Please advise on this; I cannot find the right documentation
Problem case 3: invalid redirect pointers; pruneheap.c line 949, in
heap_get_root_tuples
The heap pruning mechanism currently assumes that all redirects are
valid. Currently, if a HOT root points to !ItemIdIsNormal, it breaks
out of the loop, but doesn't actually fail. This code is already
potentially problematic because it has no bounds or sanity checking
for the nextoffnum, but with shrinking pd_linp it would also add the
failure mode of HOT tuples pointing into what is now arbitrary data.
This failure mode is now accompanied by an Assert, which fails when
the redirect is to an invalid OffsetNumber. This is not enough to not
exhibit arbitrary behaviour when accessing corrupted data with
non-assert builds, but I think that that is fine; we already do not
have a guaranteed behaviour for this failure mode.
I have also searched the contrib modules using the same method; and
all 18 usages seem to be validated correctly.
With regards,
Matthias van de Meent.
./backend/access/heap/heapam.c:678: lpp = PageGetItemId(dp, lineoff);
+ problem case 1 (table scan)
./backend/access/heap/heapam.c:811: lpp = PageGetItemId(dp,
FirstOffsetNumber);
+ problem case 1 (table scan)
./backend/access/heap/heapam.c:8796: lp =
PageGetItemId(page, xlrec_tp->offset); /* offsets are one-based */
+ Problem case 2 (xlog)
./backend/access/heap/pruneheap.c:949: lp =
PageGetItemId(page, nextoffnum);
+ Problem case 3 (invalid redirect pointers)
./backend/commands/sequence.c:1186: lp = PageGetItemId(page,
FirstOffsetNumber);
- Not a heap page (sequence)
./backend/access/brin/brin.c:283: ItemId lp =
PageGetItemId(page, off);
- Not a heap page (brin)
./backend/access/brin/brin_pageops.c:114: oldlp = PageGetItemId(oldpage,
oldoff);
- Not a heap page (brin)
./backend/access/brin/brin_pageops.c:541: lp =
PageGetItemId(page, off);
- Not a heap page (brin)
./backend/access/brin/brin_pageops.c:582: lp =
PageGetItemId(page, off);
- Not a heap page (brin)
./backend/access/brin/brin_revmap.c:296: lp =
PageGetItemId(page, *off);
- Not a heap page (brin)
./backend/access/brin/brin_revmap.c:394: lp = PageGetItemId(regPg,
regOffset);
- Not a heap page (brin)
./backend/access/gin/ginentrypage.c:240: return (IndexTuple)
PageGetItem(page, PageGetItemId(page, maxoff));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:311: itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, mid));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:336: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, high));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:382: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, mid));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:418: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, storedOff));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:428: itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, i));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:438: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, i));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:455: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, FirstOffsetNumber));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:472: IndexTuple itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, off));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:505: IndexTuple itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, off));
- Not a heap page (gin)
./backend/access/gin/ginentrypage.c:639: itup = (IndexTuple)
PageGetItem(lpage, PageGetItemId(lpage, i));
- Not a heap page (gin)
./backend/access/gin/ginfast.c:721: IndexTuple itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, i));
- Not a heap page (gin)
./backend/access/gin/ginget.c:159: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, stack->off));
- Not a heap page (gin)
./backend/access/gin/ginget.c:274: itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, stack->off));
- Not a heap page (gin)
./backend/access/gin/ginget.c:393: IndexTuple itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, stackEntry->off));
- Not a heap page (gin)
./backend/access/gin/ginget.c:1493: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, pos->firstOffset));
- Not a heap page (gin)
./backend/access/gin/ginget.c:1502: itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, pos->lastOffset));
- Not a heap page (gin)
./backend/access/gin/ginget.c:1553: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, off));
- Not a heap page (gin)
./backend/access/gin/ginget.c:1670: itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, StopMiddle));
- Not a heap page (gin)
./backend/access/gin/gininsert.c:201: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, stack->off));
- Not a heap page (gin)
./backend/access/gin/ginvacuum.c:469: IndexTuple itup =
(IndexTuple) PageGetItem(tmppage, PageGetItemId(tmppage, i));
- Not a heap page (gin)
./backend/access/gin/ginvacuum.c:539: itup =
(IndexTuple) PageGetItem(tmppage, PageGetItemId(tmppage, i));
- Not a heap page (gin)
./backend/access/gin/ginvacuum.c:629: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, FirstOffsetNumber));
- Not a heap page (gin)
./backend/access/gin/ginxlog.c:83: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offset));
- Not a heap page (gin)
./backend/access/gist/gistbuild.c:944: iid = PageGetItemId(page,
childoffnum);
- Not a heap page (gist)
./backend/access/gist/gistbuild.c:1072: ItemId
iid = PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gistbuild.c:1229: ItemId iid =
PageGetItemId(page, *downlinkoffnum);
- Not a heap page (gist)
./backend/access/gist/gistbuild.c:1248: ItemId iid =
PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gistbuild.c:1436:
PageGetItemId(page, FirstOffsetNumber));
- Not a heap page (gist)
./backend/access/gist/gistbuild.c:1528: ItemId iid =
PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gist.c:749: iid =
PageGetItemId(stack->page, downlinkoffnum);
- Not a heap page (gist)
./backend/access/gist/gist.c:984: iid =
PageGetItemId(page, i);
- Not a heap page (gist)
./backend/access/gist/gist.c:1043: iid =
PageGetItemId(parent->page, i);
- Not a heap page (gist)
./backend/access/gist/gist.c:1119: PageGetItem(page,
PageGetItemId(page, offset));
- Not a heap page (gist)
./backend/access/gist/gist.c:1150: iid =
PageGetItemId(stack->parent->page, stack->downlinkoffnum);
- Not a heap page (gist)
./backend/access/gist/gist.c:1660: ItemId itemId =
PageGetItemId(page, offnum);
- Not a heap page (gist)
./backend/access/gist/gistget.c:82: iid = PageGetItemId(page,
offnum);
- Not a heap page (gist)
./backend/access/gist/gistget.c:417: ItemId iid =
PageGetItemId(page, i);
- Not a heap page (gist)
./backend/access/gist/gistutil.c:69: IndexTuple itup =
(IndexTuple) PageGetItem(page, PageGetItemId(page, todelete));
- Not a heap page (gist)
./backend/access/gist/gistutil.c:104: itvec[i - FirstOffsetNumber] =
(IndexTuple) PageGetItem(page, PageGetItemId(page, i));
- Not a heap page (gist)
./backend/access/gist/gistutil.c:439: IndexTuple itup =
(IndexTuple) PageGetItem(p, PageGetItemId(p, i));
- Not a heap page (gist)
./backend/access/gist/gistvacuum.c:342: ItemId
iid = PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gistvacuum.c:415: ItemId iid =
PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gistvacuum.c:505: ItemId iid =
PageGetItemId(page, off);
- Not a heap page (gist)
./backend/access/gist/gistvacuum.c:628: iid = PageGetItemId(parentPage,
downlink);
- Not a heap page (gist)
./backend/access/hash/hash.c:732:
PageGetItemId(page, offno));
- Not a heap page (hash)
./backend/access/hash/hashinsert.c:354: ItemId itemId =
PageGetItemId(page, offnum);
- Not a heap page (hash)
./backend/access/hash/hashovfl.c:889: if
(ItemIdIsDead(PageGetItemId(rpage, roffnum)))
- Not a heap page (hash)
./backend/access/hash/hashovfl.c:893:
PageGetItemId(rpage, roffnum));
- Not a heap page (hash)
./backend/access/hash/hashpage.c:1128: if
(ItemIdIsDead(PageGetItemId(opage, ooffnum)))
- Not a heap page (hash)
./backend/access/hash/hashpage.c:1139:
PageGetItemId(opage, ooffnum));
- Not a heap page (hash)
./backend/access/hash/hashpage.c:1406:
PageGetItemId(npage, noffnum));
- Not a heap page (hash)
./backend/access/hash/hashsearch.c:626: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offnum));
- Not a heap page (hash)
./backend/access/hash/hashsearch.c:636:
(ItemIdIsDead(PageGetItemId(page, offnum)))))
- Not a heap page (hash)
./backend/access/hash/hashsearch.c:672: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offnum));
- Not a heap page (hash)
./backend/access/hash/hashsearch.c:682:
(ItemIdIsDead(PageGetItemId(page, offnum)))))
- Not a heap page (hash)
./backend/access/hash/hashutil.c:369: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, off));
- Not a heap page (hash)
./backend/access/hash/hashutil.c:407: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, off));
- Not a heap page (hash)
./backend/access/hash/hashutil.c:592: ItemId iid =
PageGetItemId(page, offnum);
- Not a heap page (hash)
./backend/access/index/genam.c:317: iitemid = PageGetItemId(ipage,
itemnos[i]);
- Not a heap page (generic index)
./backend/access/nbtree/nbtdedup.c:117: ItemId hitemid =
PageGetItemId(page, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtdedup.c:130: ItemId itemid =
PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtdedup.c:357: ItemId itemid =
PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtdedup.c:647: ItemId itemid =
PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtdedup.c:784: itemid = PageGetItemId(page, minoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtdedup.c:789: itemid = PageGetItemId(page,
PageGetMaxOffsetNumber(page));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:501:
curitemid = PageGetItemId(page, offset);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:1158: ItemId itemid
= PageGetItemId(page, newitemoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:1607: itemid =
PageGetItemId(origpage, firstrightoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:1631: itemid =
PageGetItemId(origpage, lastleftoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:1741: itemid =
PageGetItemId(origpage, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:1777: itemid =
PageGetItemId(origpage, i);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2013: itemid =
PageGetItemId(origpage, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2156:
PageGetItemId(page, P_HIKEY));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2351: itemid
= PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2367: itemid
= PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2466: itemid = PageGetItemId(lpage,
P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2688: ItemId itemId
= PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2810: ItemId itemid
= PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtinsert.c:2936: ItemId itemid
= PageGetItemId(page, deletable[i]);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:1532: ItemId itemid =
PageGetItemId(page, idxoffnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:1908: itemid =
PageGetItemId(page, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:2127: itemid = PageGetItemId(page, poffset);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:2133: itemid = PageGetItemId(page,
nextoffset);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:2168: itemid = PageGetItemId(page, poffset);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:2297: itemid = PageGetItemId(page, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtpage.c:2453: itemid = PageGetItemId(page,
P_FIRSTDATAKEY(opaque));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtree.c:1294:
PageGetItemId(page, offnum));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:151: itemid =
PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:577: itemid = PageGetItemId(page,
offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:669: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offnum));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:1581: ItemId
iid = PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:1648: ItemId
iid = PageGetItemId(page, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:1673: ItemId
iid = PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsearch.c:2358: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offnum));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:740: previi = PageGetItemId(rightmostpage,
P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:743: ItemId thisii =
PageGetItemId(rightmostpage, off);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:926: ii = PageGetItemId(opage,
last_off);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:940: hii = PageGetItemId(opage,
P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:973: ii =
PageGetItemId(opage, OffsetNumberPrev(last_off));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsort.c:985: hii =
PageGetItemId(opage, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:166: itemid =
PageGetItemId(origpage, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:215: itemid =
PageGetItemId(origpage, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:487: itemid =
PageGetItemId(state->origpage, firstrightoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:694: itemid =
PageGetItemId(state->origpage, maxoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:720: itemid =
PageGetItemId(state->origpage, OffsetNumberPrev(state->newitemoff));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:1033: itemid =
PageGetItemId(state->origpage, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:1150: itemid =
PageGetItemId(state->origpage, split->firstrightoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:1172: itemid =
PageGetItemId(state->origpage,
- Not a heap page (nbtree)
./backend/access/nbtree/nbtsplitloc.c:1188: itemid =
PageGetItemId(state->origpage, split->firstrightoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtutils.c:1786: ItemId
iid = PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtutils.c:2491: itup = (IndexTuple)
PageGetItem(page, PageGetItemId(page, offnum));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:218: itemid =
PageGetItemId(page, OffsetNumberPrev(xlrec->offnum));
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:351: itemid =
PageGetItemId(origpage, replacepostingoff);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:407: itemid =
PageGetItemId(origpage, off);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:505: ItemId itemid
= PageGetItemId(page, P_HIKEY);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:519: ItemId itemid
= PageGetItemId(page, offnum);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:570: itemid = PageGetItemId(page,
updatedoffsets[i]);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:746: itemid = PageGetItemId(page,
nextoffset);
- Not a heap page (nbtree)
./backend/access/nbtree/nbtxlog.c:750: itemid = PageGetItemId(page,
poffset);
- Not a heap page (nbtree)
./backend/access/spgist/spgdoinsert.c:192:
PageGetItemId(parent->page, parent->offnum));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:253:
PageGetItemId(current->page,
current->offnum));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:266:
PageGetItemId(current->page, current->offnum));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:355:
PageGetItemId(current->page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:428:
PageGetItemId(current->page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:470:
PageGetItemId(current->page,
toDelete[i]));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:573:
PageGetItemId(current->page, position));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:757:
PageGetItemId(current->page,
i));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:782:
PageGetItemId(current->page,
i));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:1837:
PageGetItemId(current->page, current->offnum));
- Not a heap page (spgist)
./backend/access/spgist/spgdoinsert.c:2111:
PageGetItemId(current.page, current.offnum));
- Not a heap page (spgist)
./backend/access/spgist/spgscan.c:718: PageGetItem(page, PageGetItemId(page,
offset));
- Not a heap page (spgist)
./backend/access/spgist/spgscan.c:845:
PageGetItem(page, PageGetItemId(page, offset));
- Not a heap page (spgist)
./backend/access/spgist/spgutils.c:908:
PageGetItemId(page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:153:
PageGetItemId(page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:242:
PageGetItemId(page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:259:
PageGetItemId(page, j));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:348: ItemId idSrc =
PageGetItemId(page, moveSrc[i]);
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:349: ItemId idDest
= PageGetItemId(page, moveDest[i]);
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:367:
PageGetItemId(page, chainSrc[i]));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:423:
PageGetItemId(page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:525: dt = (SpGistDeadTuple)
PageGetItem(page, PageGetItemId(page, i));
- Not a heap page (spgist)
./backend/access/spgist/spgvacuum.c:757:
PageGetItemId(page, offset));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:57:
PageGetItemId(page, offset));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:124:
PageGetItemId(page,
xldata->offnumHeadLeaf));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:159:
PageGetItemId(page,
xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:273:
PageGetItemId(page,
xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:367:
PageGetItemId(page, xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:414:
PageGetItemId(page, xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:438:
PageGetItemId(page, xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:706:
PageGetItemId(page,
xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:738:
PageGetItemId(page,
xldata->offnumParent));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:803: ItemId idSrc =
PageGetItemId(page, moveSrc[i]);
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:804: ItemId idDest
= PageGetItemId(page, moveDest[i]);
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:823:
PageGetItemId(page, chainSrc[i]));
- Not a heap page (spgist)
./backend/access/spgist/spgxlog.c:900:
PageGetItemId(page,
itemToPlaceholder[i]));
- Not a heap page (spgist)
./backend/access/heap/heapam.c:665: lpp = PageGetItemId(dp,
lineoff);
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:983: lpp = PageGetItemId(dp,
lineoff);
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:1005: lpp = PageGetItemId(dp,
lineoff);
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:2801: lp = PageGetItemId(page,
ItemPointerGetOffsetNumber(tid));
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:3301: lp = PageGetItemId(page,
ItemPointerGetOffsetNumber(otid));
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:4305: lp = PageGetItemId(page,
ItemPointerGetOffsetNumber(tid));
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam.c:5884: lp = PageGetItemId(page,
ItemPointerGetOffsetNumber(tid));
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/heapam_handler.c:2241: lp = PageGetItemId(dp,
targoffset);
- Has assertion ItemIdIsNormal (asserts item is not removed; so concurrent
vacuum cannot have removed this item & truncated the array)
./backend/access/heap/hio.c:78: ItemId itemId =
PageGetItemId(pageHeader, offnum);
- OffsetNumber is generated by PageAddItem
./backend/access/heap/rewriteheap.c:727: newitemid =
PageGetItemId(page, newoff);
- OffsetNumber is generated by PageAddItem
./backend/access/common/bufmask.c:105: ItemId itemId =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:449: for (lineoff = FirstOffsetNumber, lpp =
PageGetItemId(dp, lineoff);
- Has bound checks
./backend/access/heap/heapam.c:806: lpp = PageGetItemId(dp,
lines);
- Has bound checks
./backend/access/heap/heapam.c:1612: lp = PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:1721: lp = PageGetItemId(dp, offnum);
- Has bound checks
./backend/access/heap/heapam.c:1892: lp = PageGetItemId(page,
offnum);
- Has bound checks
./backend/access/heap/heapam.c:5783: lp = PageGetItemId(page,
offnum);
- Has bound checks
./backend/access/heap/heapam.c:6040: lp = PageGetItemId(page,
offnum);
- Has bound checks
./backend/access/heap/heapam.c:7515: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:8873: lp =
PageGetItemId(page, xlrec->offnum);
- Has bound checks
./backend/access/heap/heapam.c:9258: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:9469: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:9526: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:9599: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:9640: lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/heapam.c:9762: ItemId iid =
PageGetItemId(page, off);
- Has bound checks
./backend/access/heap/heapam_handler.c:1037: itemid =
PageGetItemId(targpage, hscan->rs_cindex);
- Has bound checks
./backend/access/heap/heapam_handler.c:2196: lp =
PageGetItemId(dp, offnum);
- Has bound checks
./backend/access/heap/heapam_handler.c:2381: itemid =
PageGetItemId(page, tupoffset);
- Has bound checks
./backend/access/heap/pruneheap.c:278: itemid = PageGetItemId(page,
offnum);
- Has bound checks
./backend/access/heap/pruneheap.c:575: lp = PageGetItemId(dp, offnum);
- Has bound checks
./backend/access/heap/pruneheap.c:895: ItemId lp =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/vacuumlazy.c:1271: itemid =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/vacuumlazy.c:1498: itemid
= PageGetItemId(page, frozen[i].offset);
- Has bound checks
./backend/access/heap/vacuumlazy.c:1953: itemid =
PageGetItemId(page, toff);
- Has bound checks
./backend/access/heap/vacuumlazy.c:2815: itemid =
PageGetItemId(page, offnum);
- Has bound checks
./backend/access/heap/vacuumlazy.c:3005: itemid =
PageGetItemId(page, offnum);
- Has bound checks
./backend/storage/page/bufpage.c:233: itemId =
PageGetItemId(phdr, offsetNumber);
- Has bound checks
./backend/storage/page/bufpage.c:260: itemId =
PageGetItemId(phdr, offsetNumber);
- Has bound checks
./backend/storage/page/bufpage.c:312: itemId = PageGetItemId(phdr,
offsetNumber);
- Has bound checks
./backend/storage/page/bufpage.c:724: lp = PageGetItemId(page, i);
- Has bound checks
./backend/storage/page/bufpage.c:898: ItemId
lp = PageGetItemId(page, offnum);
- Has bound checks
./backend/storage/page/bufpage.c:966: tup = PageGetItemId(page, offnum);
- Has bound checks
./backend/storage/page/bufpage.c:1025: ItemId ii =
PageGetItemId(phdr, i);
- Has bound checks
./backend/storage/page/bufpage.c:1107: lp = PageGetItemId(page,
offnum);
- Has bound checks
./backend/storage/page/bufpage.c:1204: tup = PageGetItemId(page, offnum);
- Has bound checks
./backend/storage/page/bufpage.c:1260: ItemId ii =
PageGetItemId(phdr, i);
- Has bound checks
./backend/storage/page/bufpage.c:1316: tupid = PageGetItemId(page, offnum);
- Has bound checks
./backend/storage/page/bufpage.c:1359: ItemId ii =
PageGetItemId(phdr, i);
- Has bound checks
./backend/access/heap/pruneheap.c:510: rootlp = PageGetItemId(dp, rootoffnum);
- Callsite guarantees bounds
./backend/access/heap/pruneheap.c:835: ItemId fromlp =
PageGetItemId(page, fromoff);
- Callsite guarantees bounds
./backend/access/heap/pruneheap.c:845: ItemId lp =
PageGetItemId(page, off);
- Callsite guarantees bounds
./backend/access/heap/pruneheap.c:855: ItemId lp =
PageGetItemId(page, off);
- Callsite guarantees bounds
./backend/access/heap/vacuumlazy.c:2061: itemid =
PageGetItemId(page, offnum);
- Callsite guarantees bounds
./backend/storage/page/bufpage.c:531: lp =
PageGetItemId(page, itemidptr->offsetindex + 1);
- Callsite guarantees bounds
./backend/storage/page/bufpage.c:636: lp =
PageGetItemId(page, itemidptr->offsetindex + 1);
- Callsite guarantees bounds
./include/storage/bufpage.h:232: * PageGetItemId
- Not a call
./include/storage/bufpage.h:235:#define PageGetItemId(page, offsetNumber) \
- Not a call
./backend/access/brin/brin_pageops.c:121: * PageGetItemId() is simple
enough that it was safe to do that
- Not a call
./backend/storage/page/bufpage.c:984: * PageGetItemId, because we are
manipulating the _array_, not individual
- Not a call
From f49e1bb31f4ad56c3f42b9ed4e9fcb5d99198ed4 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm@gmail.com>
Date: Tue, 9 Mar 2021 14:42:52 +0100
Subject: [PATCH v3] Truncate a pages' line pointer array when it has trailing
unused ItemIds.
This will allow reuse of what is effectively free space for data as well as
new line pointers, instead of keeping it reserved for line pointers only.
An additional benefit is that the HasFreeLinePointers hint-bit optimization
now doesn't hint for free line pointers at the end of the array, slightly
increasing the specificity of where the free lines are; and saving us from
needing to search to the end of the array if all other entries are already
filled.
---
src/backend/access/heap/heapam.c | 9 ++++++++-
src/backend/access/heap/pruneheap.c | 1 +
src/backend/storage/page/bufpage.c | 13 ++++++++++++-
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 3b435c107d..ae218cfac6 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -635,8 +635,15 @@ heapgettup(HeapScanDesc scan,
}
else
{
+ /*
+ * The previous returned tuple may have been vacuumed since the
+ * previous scan when we use a non-MVCC snapshot, so we must
+ * re-establish the lineoff <= PageGetMaxOffsetNumber(dp)
+ * invariant
+ */
lineoff = /* previous offnum */
- OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self)));
+ Min(lines,
+ OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self))));
}
/* page and lineoff now reference the physically previous tid */
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 8bb38d6406..5903cdca82 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -946,6 +946,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
*/
for (;;)
{
+ Assert(OffsetNumberIsValid(nextoffnum) && nextoffnum <= maxoff);
lp = PageGetItemId(page, nextoffnum);
/* Check for broken chains */
diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index 9ac556b4ae..10d8f26ad0 100644
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -672,7 +672,11 @@ compactify_tuples(itemIdCompact itemidbase, int nitems, Page page, bool presorte
* PageRepairFragmentation
*
* Frees fragmented space on a page.
- * It doesn't remove unused line pointers! Please don't change this.
+ * It doesn't remove intermediate unused line pointers (that would mean
+ * moving ItemIds, and that would imply invalidating indexed values), but it
+ * does truncate the page->pd_linp array to the last unused line pointer, so
+ * that this space may also be reused for data, instead of only for line
+ * pointers.
*
* This routine is usable for heap pages only, but see PageIndexMultiDelete.
*
@@ -691,6 +695,7 @@ PageRepairFragmentation(Page page)
int nline,
nstorage,
nunused;
+ OffsetNumber lastUsed = InvalidOffsetNumber;
int i;
Size totallen;
bool presorted = true; /* For now */
@@ -724,6 +729,7 @@ PageRepairFragmentation(Page page)
lp = PageGetItemId(page, i);
if (ItemIdIsUsed(lp))
{
+ lastUsed = i;
if (ItemIdHasStorage(lp))
{
itemidptr->offsetindex = i - 1;
@@ -771,6 +777,11 @@ PageRepairFragmentation(Page page)
compactify_tuples(itemidbase, nstorage, page, presorted);
}
+ if (lastUsed != nline) {
+ ((PageHeader) page)->pd_lower = SizeOfPageHeaderData + (sizeof(ItemIdData) * lastUsed);
+ nunused = nunused - (nline - lastUsed);
+ }
+
/* Set hint bit for PageAddItem */
if (nunused > 0)
PageSetHasFreeLinePointers(page);
--
2.20.1
