Re: [HACKERS] A design for amcheck heapam verification

On Thu, Mar 29, 2018 at 7:42 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>> We should be able to get this into v11...
>
> That's a relief. Thanks.

I have a new revision lined up. I won't send anything to the list
until you clear up what you meant in those few cases where it seemed
unclear.

I also acted on some of the feedback from Pavan, which I'd previously
put off/deferred. It seemed like there was no reason not to do it his
way when it came to minor stylistic points that were non-issues to me.

Finally, I added something myself:

745 /*
746 * lp_len should match the IndexTuple reported length
exactly, since
747 * lp_len is completely redundant in indexes, and both
sources of tuple
748 * length are MAXALIGN()'d. nbtree does not use lp_len all that
749 * frequently, and is surprisingly tolerant of corrupt
lp_len fields.
750 */
751 if (tupsize != ItemIdGetLength(itemid))
752 ereport(ERROR,
753 (errcode(ERRCODE_INDEX_CORRUPTED),
754 errmsg("index tuple size does not equal
lp_len in index \"%s\"",
755 RelationGetRelationName(state->rel)),
756 errdetail_internal("Index tid=(%u,%u) tuple
size=%zu lp_len=%u page lsn=%X/%X.",
757 state->targetblock, offset,
758 tupsize, ItemIdGetLength(itemid),
759 (uint32) (state->targetlsn >> 32),
760 (uint32) state->targetlsn),
761 errhint("This could be a torn page problem")));

It seems to me that we should take the opportunity to verify each
tuple's IndexTupleSize() value, now that we'll be using it directly.
There happens to be an easy way to do that, so why not just do it?

This is unlikely to find an error that we wouldn't have detected
anyway, even without using the new heapallindexed option. However, it
seems likely that this error message is more accurate in the real
world cases where it will be seen. A torn page can leave us with a
page image that looks surprisingly not-so-corrupt.

--
Peter Geoghegan