amcheck: Reword some messages and fix an alignment problem.

robertmhaas · robertmhaas · commit 9acaf1a62197 · 2021-04-14T12:46:31.000-04:00
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
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
@@ -1179,15 +1179,17 @@ 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,
 										ctx->toast_rel->rd_att, &isnull));
 	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,21 +1374,26 @@ 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;
 	}
 
 	/* The relation better have a toast table */
 	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
@@ -480,7 +480,7 @@ sub header
 
 		$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 @@ sub header
 
 		$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)
 	{

Original file line number	Diff line number	Diff line change
`@@ -480,7 +480,7 @@ sub header`
`480`	`480`
`481`	`481`	`$header = header(0, $offnum, 1);`
`482`	`482`	`push @expected,`
`483`		`- qr/${header}attribute \d+ with length \d+ ends at offset \d+ beyond total tuple length \d+/;`
	`483`	`+ qr/${header}attribute with length \d+ ends at offset \d+ beyond total tuple length \d+/;`
`484`	`484`	`}`
`485`	`485`	`elsif ($offnum == 13)`
`486`	`486`	`{`
`@@ -489,7 +489,7 @@ sub header`
`489`	`489`
`490`	`490`	`$header = header(0, $offnum, 2);`
`491`	`491`	`push @expected,`
`492`		`- qr/${header}toasted value for attribute 2 missing from toast table/;`
	`492`	`+ qr/${header}toast value \d+ not found in toast table/;`
`493`	`493`	`}`
`494`	`494`	`elsif ($offnum == 14)`
`495`	`495`	`{`