diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index a8ed11a1858..2f477aa43b1 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -143,7 +143,7 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum, bool was_normal); static void heap_prune_record_unchanged(Page page, PruneState *prstate, OffsetNumber offnum); -static void heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum); +static void heap_prune_record_unchanged_lp_dead(ItemId itemid, PruneState *prstate, OffsetNumber offnum); static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetNumber offnum); static void page_verify_redirects(Page page); @@ -766,7 +766,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer, * we can advance relfrozenxid and relminmxid to the values in * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid. * MFIXME: which one should be pick if presult->nfrozen == 0 and - * presult->all_frozen = True. + * presult->all_frozen = True. MTODO: see Peter's response here + * https://www.postgresql.org/message-id/CAH2-Wz%3DLmOs%3DiJ%3DFfCERnma0q7QjaNSnCgWEp7zOK7hD24YC_w%40mail.gmail.com */ if (new_relfrozen_xid) { @@ -868,9 +869,12 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, OffsetNumber latestdead = InvalidOffsetNumber, maxoff = PageGetMaxOffsetNumber(dp), offnum; + + /* TODO: while maybe self-explanatory, I would prefer if chainitems and */ + /* nchain had a comment up here */ OffsetNumber chainitems[MaxHeapTuplesPerPage]; - int nchain = 0, - i; + int nchain = 0; + int i = 0; rootlp = PageGetItemId(dp, rootoffnum); @@ -943,6 +947,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, */ prstate->revisit[prstate->nrevisit++] = rootoffnum; + /* TODO: I don't like this comment now */ /* Nothing more to do */ return; } @@ -1020,7 +1025,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, if (unlikely(prstate->actions & PRUNE_DO_MARK_UNUSED_NOW)) heap_prune_record_unused(prstate, offnum, false); else - heap_prune_record_unchanged_lp_dead(prstate, offnum); + heap_prune_record_unchanged_lp_dead(lp, prstate, offnum); break; } @@ -1044,6 +1049,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, */ tupdead = recent_dead = false; + /* TODO: maybe this should just be an if statement now */ switch (htsv_get_valid_status(prstate->htsv[offnum])) { case HEAPTUPLE_DEAD: @@ -1132,42 +1138,34 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum, * dead and the root line pointer can be marked dead. Otherwise just * redirect the root to the correct chain member. */ - if (i >= nchain) - heap_prune_record_dead_or_unused(prstate, rootoffnum, ItemIdIsNormal(rootlp)); - else - { + if (i < nchain) heap_prune_record_redirect(prstate, rootoffnum, chainitems[i], ItemIdIsNormal(rootlp)); - - /* the rest of tuples in the chain are normal, unchanged tuples */ - for (; i < nchain; i++) - heap_prune_record_unchanged(dp, prstate, chainitems[i]); - } + else + heap_prune_record_dead_or_unused(prstate, rootoffnum, ItemIdIsNormal(rootlp)); } - else + else if ((i = ItemIdIsRedirected(rootlp))) { - i = 0; - if (ItemIdIsRedirected(rootlp)) + if (i < nchain) { - if (nchain < 2) - { - /* - * We found a redirect item that doesn't point to a valid - * follow-on item. This can happen if the loop in - * heap_page_prune_and_freeze() caused us to visit the dead - * successor of a redirect item before visiting the redirect - * item. We can clean up by setting the redirect item to DEAD - * state or LP_UNUSED if the caller indicated. - */ - heap_prune_record_dead_or_unused(prstate, rootoffnum, false); - } - else - heap_prune_record_unchanged_lp_redirect(prstate, rootoffnum); - i++; + heap_prune_record_unchanged_lp_redirect(prstate, rootoffnum); + } + else + { + /* + * We found a redirect item that doesn't point to a valid + * follow-on item. This can happen if the loop in + * heap_page_prune_and_freeze() caused us to visit the dead + * successor of a redirect item before visiting the redirect item. + * We can clean up by setting the redirect item to DEAD state or + * LP_UNUSED if the caller indicated. + */ + heap_prune_record_dead_or_unused(prstate, rootoffnum, false); } - /* the rest of tuples in the chain are normal, unchanged tuples */ - for (; i < nchain; i++) - heap_prune_record_unchanged(dp, prstate, chainitems[i]); } + + /* the rest of tuples in the chain are normal, unchanged tuples */ + for (; i < nchain; i++) + heap_prune_record_unchanged(dp, prstate, chainitems[i]); } /* Record lowest soon-prunable XID */ @@ -1486,7 +1484,7 @@ heap_prune_record_unchanged(Page page, PruneState *prstate, OffsetNumber offnum) * Record line pointer that was already LP_DEAD and is left unchanged. */ static void -heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum) +heap_prune_record_unchanged_lp_dead(ItemId itemid, PruneState *prstate, OffsetNumber offnum) { /* * Deliberately don't set hastup for LP_DEAD items. We make the soft @@ -1506,6 +1504,7 @@ heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum) Assert(!prstate->marked[offnum]); prstate->marked[offnum] = true; + Assert(!ItemIdHasStorage(itemid)); prstate->deadoffsets[prstate->lpdead_items++] = offnum; }