From b6ea48417cc99258bf2f9106feed4bfebfcf8212 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Thu, 8 Apr 2021 09:48:37 -0700 Subject: [PATCH v19 1/3] amcheck: rewording messages and fixing alignment Removing redundant mention of attnum in the corruption message text, as the attnum is already its own separate column. When reporting toast corruption, mentioning the toast value in the message since that information is not otherwise reported. Being more careful about alignment when accessing a toast pointer. --- 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 36607596b1..307f14611c 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) { -- 2.21.1 (Apple Git-122.3)