Re: HOT chain validation in verify_heapam()

On Thu, Mar 23, 2023 at 1:26 PM Andres Freund <andres@anarazel.de> wrote:
> E.g. continuing after:
>
>                                 rditem = PageGetItemId(ctx.page, rdoffnum);
>                                 if (!ItemIdIsUsed(rditem))
>                                         report_corruption(&ctx,
>                                                                           psprintf("line pointer redirection to unused item at offset %u",
>                                                                                            (unsigned) rdoffnum));
>
> means we'll look into the tuple in the "update chain validation" loop for
> unused items.

Ah, yes, that's a goof for sure.

> > -                 * Redirects are created by updates, so successor should be
> > -                 * the result of an update.
> > +                 * Redirects are created by HOT updates, so successor should
> > +                 * be the result of an HOT update.
> > +                 *
> > +                 * XXX: HeapTupleHeaderIsHeapOnly() should always imply
> > +                 * HEAP_UPDATED. This should be checked even when the tuple
> > +                 * isn't a target of a redirect.
> >
> > Hmm, OK. So the question is where to put this check. Maybe inside
> > check_tuple_header(), making it independent of the update chain
> > validation stuff?
>
> Yes, check_tuple_header sounds sensible to me.

OK, let me spend some more time on this and I'll post a patch (or
patches) in a bit.

-- 
Robert Haas
EDB: http://www.enterprisedb.com