amcheck: Improve some confusing reports about TOAST problems.

Don't phrase reports in terms of the number of tuples thus-far
returned by the index scan, but rather in terms of the chunk_seq
values found inside the tuples.

Patch by me, reviewed by Mark Dilger.

Discussion: http://postgr.es/m/CA+TgmoZUONCkdcdR778EKuE+f1r5Obieu63db2OgMPHaEvEPTQ@mail.gmail.com

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 9f159eb3dbd8dd792965658f9fa7518016441942..36c1b791a22d9f4fc1071ea5a18f21ef27194ee2 100644 (file)
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -150,8 +150,8 @@ typedef struct HeapCheckContext
 static void sanity_check_relation(Relation rel);
 static void check_tuple(HeapCheckContext *ctx);
 static void check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
-                             ToastedAttribute *ta, int32 chunkno,
-                             int32 endchunk);
+                             ToastedAttribute *ta, int32 *expected_chunk_seq,
+                             uint32 extsize);
 
 static bool check_tuple_attribute(HeapCheckContext *ctx);
 static void check_toasted_attribute(HeapCheckContext *ctx,
@@ -1159,23 +1159,25 @@ check_tuple_visibility(HeapCheckContext *ctx)
  * each toast tuple being checked against where we are in the sequence, as well
  * as each toast tuple having its varlena structure sanity checked.
  *
- * Returns whether the toast tuple passed the corruption checks.
+ * On entry, *expected_chunk_seq should be the chunk_seq value that we expect
+ * to find in toasttup. On exit, it will be updated to the value the next call
+ * to this function should expect to see.
  */
 static void
 check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
-                 ToastedAttribute *ta, int32 chunkno, int32 endchunk)
+                 ToastedAttribute *ta, int32 *expected_chunk_seq,
+                 uint32 extsize)
 {
-   int32       curchunk;
+   int32       chunk_seq;
+   int32       last_chunk_seq = (extsize - 1) / TOAST_MAX_CHUNK_SIZE;
    Pointer     chunk;
    bool        isnull;
    int32       chunksize;
    int32       expected_size;
 
-   /*
-    * Have a chunk, extract the sequence number and the data
-    */
-   curchunk = DatumGetInt32(fastgetattr(toasttup, 2,
-                                        ctx->toast_rel->rd_att, &isnull));
+   /* Sanity-check the sequence number. */
+   chunk_seq = DatumGetInt32(fastgetattr(toasttup, 2,
+                                         ctx->toast_rel->rd_att, &isnull));
    if (isnull)
    {
        report_toast_corruption(ctx, ta,
@@ -1183,13 +1185,25 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
                                         ta->toast_pointer.va_valueid));
        return;
    }
+   if (chunk_seq != *expected_chunk_seq)
+   {
+       /* Either the TOAST index is corrupt, or we don't have all chunks. */
+       report_toast_corruption(ctx, ta,
+                               psprintf("toast value %u index scan returned chunk %d when expecting chunk %d",
+                                        ta->toast_pointer.va_valueid,
+                                        chunk_seq, *expected_chunk_seq));
+   }
+   *expected_chunk_seq = chunk_seq + 1;
+
+   /* Sanity-check the chunk data. */
    chunk = DatumGetPointer(fastgetattr(toasttup, 3,
                                        ctx->toast_rel->rd_att, &isnull));
    if (isnull)
    {
        report_toast_corruption(ctx, ta,
                                psprintf("toast value %u chunk %d has null data",
-                                        ta->toast_pointer.va_valueid, chunkno));
+                                        ta->toast_pointer.va_valueid,
+                                        chunk_seq));
        return;
    }
    if (!VARATT_IS_EXTENDED(chunk))
@@ -1209,40 +1223,31 @@ check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
        report_toast_corruption(ctx, ta,
                                psprintf("toast value %u chunk %d has invalid varlena header %0x",
                                         ta->toast_pointer.va_valueid,
-                                        chunkno, header));
+                                        chunk_seq, header));
        return;
    }
 
    /*
     * Some checks on the data we've found
     */
-   if (curchunk != chunkno)
-   {
-       report_toast_corruption(ctx, ta,
-                               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 (chunkno > endchunk)
+   if (chunk_seq > last_chunk_seq)
    {
        report_toast_corruption(ctx, ta,
                                psprintf("toast value %u chunk %d follows last expected chunk %d",
                                         ta->toast_pointer.va_valueid,
-                                        chunkno, endchunk));
+                                        chunk_seq, last_chunk_seq));
        return;
    }
 
-   expected_size = curchunk < endchunk ? TOAST_MAX_CHUNK_SIZE
-       : VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - (endchunk * TOAST_MAX_CHUNK_SIZE);
+   expected_size = chunk_seq < last_chunk_seq ? TOAST_MAX_CHUNK_SIZE
+       : extsize - (last_chunk_seq * TOAST_MAX_CHUNK_SIZE);
 
    if (chunksize != expected_size)
        report_toast_corruption(ctx, ta,
                                psprintf("toast value %u chunk %d has size %u, but expected size %u",
                                         ta->toast_pointer.va_valueid,
-                                        chunkno, chunksize, expected_size));
+                                        chunk_seq, chunksize, expected_size));
 }
-
 /*
  * Check the current attribute as tracked in ctx, recording any corruption
  * found in ctx->tupstore.
@@ -1436,10 +1441,12 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
    SysScanDesc toastscan;
    bool        found_toasttup;
    HeapTuple   toasttup;
-   int32       chunkno;
-   int32       endchunk;
+   uint32      extsize;
+   int32       expected_chunk_seq = 0;
+   int32       last_chunk_seq;
 
-   endchunk = (VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer) - 1) / TOAST_MAX_CHUNK_SIZE;
+   extsize = VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer);
+   last_chunk_seq = (extsize - 1) / TOAST_MAX_CHUNK_SIZE;
 
    /*
     * Setup a scan key to find chunks in toast table with matching va_valueid
@@ -1458,15 +1465,13 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
                                           ctx->valid_toast_index,
                                           &SnapshotToast, 1,
                                           &toastkey);
-   chunkno = 0;
    found_toasttup = false;
    while ((toasttup =
            systable_getnext_ordered(toastscan,
                                     ForwardScanDirection)) != NULL)
    {
        found_toasttup = true;
-       check_toast_tuple(toasttup, ctx, ta, chunkno, endchunk);
-       chunkno++;
+       check_toast_tuple(toasttup, ctx, ta, &expected_chunk_seq, extsize);
    }
    systable_endscan_ordered(toastscan);
 
@@ -1474,11 +1479,11 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta)
        report_toast_corruption(ctx, ta,
                                psprintf("toast value %u not found in toast table",
                                         ta->toast_pointer.va_valueid));
-   else if (chunkno != (endchunk + 1))
+   else if (expected_chunk_seq <= last_chunk_seq)
        report_toast_corruption(ctx, ta,
-                               psprintf("toast value %u was expected to end at chunk %d, but ended at chunk %d",
+                               psprintf("toast value %u was expected to end at chunk %d, but ended while expecting chunk %d",
                                         ta->toast_pointer.va_valueid,
-                                        (endchunk + 1), chunkno));
+                                        last_chunk_seq, expected_chunk_seq));
 }
 
 /*