From 9acaf1a62197205b06a85afbfcaa7ffaac939ef3 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 14 Apr 2021 12:46:31 -0400 Subject: [PATCH] amcheck: Reword some messages and fix an alignment problem. We don't need to mention the attribute number in these messages, because there's a dedicated column for that, but we should mention the toast value ID, because that's really useful for any follow-up troubleshooting the user wants to do. This also rewords some of the messages to hopefully read a little better. Also, use VARATT_EXTERNAL_GET_POINTER in case we're accessing a TOAST pointer that isn't aligned on a platform that's fussy about alignment, so that we don't crash while corruption-checking the user's data. Mark Dilger, reviewed by me. Discussion: http://postgr.es/m/7D3B9BF6-50D0-4C30-8506-1C1851C7F96F@enterprisedb.com --- contrib/amcheck/verify_heapam.c | 63 +++++++++++++---------- src/bin/pg_amcheck/t/004_verify_heapam.pl | 4 +- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index e8aa0d68d4..13f420d9ad 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1179,7 +1179,8 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, if (isnull) { report_toast_corruption(ctx, ta, - pstrdup("toast chunk sequence number is null")); + psprintf("toast value %u has toast chunk with null sequence number", + ta->toast_pointer.va_valueid)); return; } chunk = DatumGetPointer(fastgetattr(toasttup, 3, @@ -1187,7 +1188,8 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, if (isnull) { report_toast_corruption(ctx, ta, - pstrdup("toast chunk data is null")); + psprintf("toast value %u chunk %d has null data", + ta->toast_pointer.va_valueid, chunkno)); return; } if (!VARATT_IS_EXTENDED(chunk)) @@ -1205,8 +1207,9 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, uint32 header = ((varattrib_4b *) chunk)->va_4byte.va_header; report_toast_corruption(ctx, ta, - psprintf("corrupt extended toast chunk has invalid varlena header: %0x (sequence number %d)", - header, curchunk)); + psprintf("toast value %u chunk %d has invalid varlena header %0x", + ta->toast_pointer.va_valueid, + chunkno, header)); return; } @@ -1216,15 +1219,17 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, if (curchunk != chunkno) { report_toast_corruption(ctx, ta, - psprintf("toast chunk sequence number %u does not match the expected sequence number %u", - curchunk, chunkno)); + psprintf("toast value %u chunk %d has sequence number %d, but expected sequence number %d", + ta->toast_pointer.va_valueid, + chunkno, curchunk, chunkno)); return; } - if (curchunk > endchunk) + if (chunkno > endchunk) { report_toast_corruption(ctx, ta, - psprintf("toast chunk sequence number %u exceeds the end chunk sequence number %u", - curchunk, endchunk)); + psprintf("toast value %u chunk %d follows last expected chunk %d", + ta->toast_pointer.va_valueid, + chunkno, endchunk)); return; } @@ -1233,8 +1238,9 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx, if (chunksize != expected_size) report_toast_corruption(ctx, ta, - psprintf("toast chunk size %u differs from the expected size %u", - chunksize, expected_size)); + psprintf("toast value %u chunk %d has size %u, but expected size %u", + ta->toast_pointer.va_valueid, + chunkno, chunksize, expected_size)); } /* @@ -1265,6 +1271,7 @@ check_tuple_attribute(HeapCheckContext *ctx) char *tp; /* pointer to the tuple data */ uint16 infomask; Form_pg_attribute thisatt; + struct varatt_external toast_pointer; infomask = ctx->tuphdr->t_infomask; thisatt = TupleDescAttr(RelationGetDescr(ctx->rel), ctx->attnum); @@ -1274,8 +1281,7 @@ check_tuple_attribute(HeapCheckContext *ctx) if (ctx->tuphdr->t_hoff + ctx->offset > ctx->lp_len) { report_corruption(ctx, - psprintf("attribute %u with length %u starts at offset %u beyond total tuple length %u", - ctx->attnum, + psprintf("attribute with length %u starts at offset %u beyond total tuple length %u", thisatt->attlen, ctx->tuphdr->t_hoff + ctx->offset, ctx->lp_len)); @@ -1295,8 +1301,7 @@ check_tuple_attribute(HeapCheckContext *ctx) if (ctx->tuphdr->t_hoff + ctx->offset > ctx->lp_len) { report_corruption(ctx, - psprintf("attribute %u with length %u ends at offset %u beyond total tuple length %u", - ctx->attnum, + psprintf("attribute with length %u ends at offset %u beyond total tuple length %u", thisatt->attlen, ctx->tuphdr->t_hoff + ctx->offset, ctx->lp_len)); @@ -1328,8 +1333,7 @@ check_tuple_attribute(HeapCheckContext *ctx) if (va_tag != VARTAG_ONDISK) { report_corruption(ctx, - psprintf("toasted attribute %u has unexpected TOAST tag %u", - ctx->attnum, + psprintf("toasted attribute has unexpected TOAST tag %u", va_tag)); /* We can't know where the next attribute begins */ return false; @@ -1343,8 +1347,7 @@ check_tuple_attribute(HeapCheckContext *ctx) if (ctx->tuphdr->t_hoff + ctx->offset > ctx->lp_len) { report_corruption(ctx, - psprintf("attribute %u with length %u ends at offset %u beyond total tuple length %u", - ctx->attnum, + psprintf("attribute with length %u ends at offset %u beyond total tuple length %u", thisatt->attlen, ctx->tuphdr->t_hoff + ctx->offset, ctx->lp_len)); @@ -1371,12 +1374,17 @@ check_tuple_attribute(HeapCheckContext *ctx) /* It is external, and we're looking at a page on disk */ + /* + * Must copy attr into toast_pointer for alignment considerations + */ + VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); + /* The tuple header better claim to contain toasted values */ if (!(infomask & HEAP_HASEXTERNAL)) { report_corruption(ctx, - psprintf("attribute %u is external but tuple header flag HEAP_HASEXTERNAL not set", - ctx->attnum)); + psprintf("toast value %u is external but tuple header flag HEAP_HASEXTERNAL not set", + toast_pointer.va_valueid)); return true; } @@ -1384,8 +1392,8 @@ check_tuple_attribute(HeapCheckContext *ctx) if (!ctx->rel->rd_rel->reltoastrelid) { report_corruption(ctx, - psprintf("attribute %u is external but relation has no toast relation", - ctx->attnum)); + psprintf("toast value %u is external but relation has no toast relation", + toast_pointer.va_valueid)); return true; } @@ -1464,12 +1472,13 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) if (!found_toasttup) report_toast_corruption(ctx, ta, - psprintf("toasted value for attribute %u missing from toast table", - ta->attnum)); + psprintf("toast value %u not found in toast table", + ta->toast_pointer.va_valueid)); else if (chunkno != (endchunk + 1)) report_toast_corruption(ctx, ta, - psprintf("final toast chunk number %u differs from expected value %u", - chunkno, (endchunk + 1))); + psprintf("toast value %u was expected to end at chunk %u, but ended at chunk %u", + ta->toast_pointer.va_valueid, + (endchunk + 1), chunkno)); } /* diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl index 2171d236a7..3c1277adf3 100644 --- a/src/bin/pg_amcheck/t/004_verify_heapam.pl +++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl @@ -480,7 +480,7 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++) $header = header(0, $offnum, 1); push @expected, - qr/${header}attribute \d+ with length \d+ ends at offset \d+ beyond total tuple length \d+/; + qr/${header}attribute with length \d+ ends at offset \d+ beyond total tuple length \d+/; } elsif ($offnum == 13) { @@ -489,7 +489,7 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++) $header = header(0, $offnum, 2); push @expected, - qr/${header}toasted value for attribute 2 missing from toast table/; + qr/${header}toast value \d+ not found in toast table/; } elsif ($offnum == 14) {