mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

amcheck: Generalize one of the recently-added update chain checks.

Browse Source 
Commit bbc1376b39627c6bddd8a0dc0a7dda24c91a97a0 checked that if
a redirected line pointer pointed to a tuple, the tuple should be
marked both HEAP_ONLY_TUPLE and HEAP_UPDATED. But Andres Freund
pointed out that *any* tuple that is marked HEAP_ONLY_TUPLE should
be marked HEAP_UPDATED, not just one that is the target of a
redirected line pointer. Do that instead.

To see why this is better, consider a redirect line pointer A
which points to a heap-only tuple B which points (via CTID)
to another heap-only tuple C. With the old code, we'd complain
if B was not marked HEAP_UPDATED, but with this change, we'll
complain if either B or C is not marked HEAP_UPDATED.

(Note that, with or without this commit, if either B or C were
not marked HEAP_ONLY_TUPLE, we would also complain about that.)

Discussion: http://postgr.es/m/CA%2BTgmobLypZx%3DcOH%2ByY1GZmCruaoucHm77A6y_-Bo%3Dh-_3H28g%40mail.gmail.com
This commit is contained in:
Robert Haas 2023-03-27 13:37:16 -04:00
parent 80d5e3a615
commit c87aff065c
2 changed files with 14 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
contrib/amcheck/verify_heapam.c
View File

@ -624,17 +624,6 @@ verify_heapam(PG_FUNCTION_ARGS)
(unsigned) nextoffnum)); (unsigned) nextoffnum));
} }
/*
* Redirects are created by updates, so successor should be
* the result of an update.
*/
if ((next_htup->t_infomask & HEAP_UPDATED) == 0)
{
report_corruption(&ctx,
psprintf("redirected line pointer points to a non-heap-updated tuple at offset %u",
(unsigned) nextoffnum));
}
/* HOT chains should not intersect. */ /* HOT chains should not intersect. */
if (predecessor[nextoffnum] != InvalidOffsetNumber) if (predecessor[nextoffnum] != InvalidOffsetNumber)
{ {
@ -954,6 +943,15 @@ check_tuple_header(HeapCheckContext *ctx)
*/ */
} }
if (HeapTupleHeaderIsHeapOnly(tuphdr) &&
((tuphdr->t_infomask & HEAP_UPDATED) == 0))
{
report_corruption(ctx,
psprintf("tuple is heap only, but not the result of an update"));
/* Here again, we can still perform further checks. */
}
if (infomask & HEAP_HASNULL) if (infomask & HEAP_HASNULL)
expected_hoff = MAXALIGN(SizeofHeapTupleHeader + BITMAPLEN(ctx->natts)); expected_hoff = MAXALIGN(SizeofHeapTupleHeader + BITMAPLEN(ctx->natts));
else else

11
src/bin/pg_amcheck/t/004_verify_heapam.pl
View File

@ -617,12 +617,10 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
} }
elsif ($offnum == 17) elsif ($offnum == 17)
{ {
# at offnum 19 we will unset HEAP_ONLY_TUPLE and HEAP_UPDATED flags. # at offnum 19 we will unset HEAP_ONLY_TUPLE flag
die "offnum $offnum should be a redirect" if defined $tup; die "offnum $offnum should be a redirect" if defined $tup;
push @expected, push @expected,
qr/${header}redirected line pointer points to a non-heap-only tuple at offset \d+/; qr/${header}redirected line pointer points to a non-heap-only tuple at offset \d+/;
push @expected,
qr/${header}redirected line pointer points to a non-heap-updated tuple at offset \d+/;
} }
elsif ($offnum == 18) elsif ($offnum == 18)
{ {
@ -637,10 +635,9 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
} }
elsif ($offnum == 19) elsif ($offnum == 19)
{ {
# unset HEAP_ONLY_TUPLE and HEAP_UPDATED flag, so that update chain # unset HEAP_ONLY_TUPLE flag, so that update chain validation will
# validation will complain about offset 17 # complain about offset 17
$tup->{t_infomask2} &= ~HEAP_ONLY_TUPLE; $tup->{t_infomask2} &= ~HEAP_ONLY_TUPLE;
$tup->{t_infomask} &= ~HEAP_UPDATED;
} }
elsif ($offnum == 22) elsif ($offnum == 22)
{ {
@ -688,6 +685,8 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
$tup->{t_infomask2} |= HEAP_ONLY_TUPLE; $tup->{t_infomask2} |= HEAP_ONLY_TUPLE;
push @expected, push @expected,
qr/${header}tuple is root of chain but is marked as heap-only tuple/; qr/${header}tuple is root of chain but is marked as heap-only tuple/;
push @expected,
qr/${header}tuple is heap only, but not the result of an update/;
} }
elsif ($offnum == 33) elsif ($offnum == 33)
{ {