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

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

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 1dc9d89f306cd253860208e8692d68d9b7420bf5..ce3114f4e6c4136d7ccb24d64a397bb2b9145ce3 100644 (file)
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -624,17 +624,6 @@ verify_heapam(PG_FUNCTION_ARGS)
                                                                                           (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. */
                                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)
                expected_hoff = MAXALIGN(SizeofHeapTupleHeader + BITMAPLEN(ctx->natts));
        else

diff --git a/src/bin/pg_amcheck/t/004_verify_heapam.pl b/src/bin/pg_amcheck/t/004_verify_heapam.pl
index cc4aa4af875b3a6ff916e87a04374c35e1a6c2ae..90ba59ec0b7142db58647cae9e178bd812ff19ff 100644 (file)
--- a/src/bin/pg_amcheck/t/004_verify_heapam.pl
+++ b/src/bin/pg_amcheck/t/004_verify_heapam.pl
@@ -617,12 +617,10 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
        }
        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;
                push @expected,
                        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)
        {
@@ -637,10 +635,9 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
        }
        elsif ($offnum == 19)
        {
-               # unset HEAP_ONLY_TUPLE and HEAP_UPDATED flag, so that update chain
-               # validation will complain about offset 17
+               # unset HEAP_ONLY_TUPLE flag, so that update chain validation will
+               # complain about offset 17
                $tup->{t_infomask2} &= ~HEAP_ONLY_TUPLE;
-               $tup->{t_infomask} &= ~HEAP_UPDATED;
        }
        elsif ($offnum == 22)
        {
@@ -688,6 +685,8 @@ for (my $tupidx = 0; $tupidx < $ROWCOUNT; $tupidx++)
                $tup->{t_infomask2} |= HEAP_ONLY_TUPLE;
                push @expected,
                  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)
        {