From 4195bb5d6e39ec0cc2b323b0ad5b24c4c843df88 Mon Sep 17 00:00:00 2001 From: Mark Dilger Date: Fri, 2 Apr 2021 15:06:50 -0700 Subject: [PATCH v18 3/4] amcheck: improving corruption messages. 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. --- contrib/amcheck/verify_heapam.c | 61 +++++++++++++---------- src/bin/pg_amcheck/t/004_verify_heapam.pl | 4 +- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index d8a3e966c4..cd1f2c4113 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 data is null", + ta->toast_pointer.va_valueid)); 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 corrupt extended chunk has invalid varlena header: %0x (sequence number %d)", + ta->toast_pointer.va_valueid, + header, curchunk)); 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 sequence number %u does not match the expected sequence number %u", + ta->toast_pointer.va_valueid, + curchunk, chunkno)); return; } if (curchunk > 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 sequence number %u exceeds the end chunk sequence number %u", + ta->toast_pointer.va_valueid, + curchunk, 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 size %u differs from the expected size %u", + ta->toast_pointer.va_valueid, + 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)