From bdce2488d8e17908b1baf056c943825d4677ab3e Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Sun, 23 Aug 2026 20:13:47 +0000 Subject: [PATCH v2 5/8] Add amcheck verification for Direct TOAST tuples Enhance verify_heapam in contrib/amcheck to validate direct TOAST pointers: - Validate that the target TID referenced by va_tid resides on a valid block and offset in the toast table. - Verify chunk header integrity (chunk_id is NULL, chunk_seq >= 0). - Recursively validate child chunk TIDs and byte offset monotonicity for hierarchical direct TOAST trees. - Check payload byte count consistency against va_extinfo. - Add test coverage in check_heap.sql. --- contrib/amcheck/expected/check_heap.out | 9 ++ contrib/amcheck/sql/check_heap.sql | 6 + contrib/amcheck/verify_heapam.c | 183 +++++++++++++++++++++++- 3 files changed, 197 insertions(+), 1 deletion(-) diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out index 979e5e84e72..ed3bd7a79d0 100644 --- a/contrib/amcheck/expected/check_heap.out +++ b/contrib/amcheck/expected/check_heap.out @@ -231,6 +231,15 @@ SELECT * FROM verify_heapam('test_foreign_table', endblock := NULL); ERROR: cannot check relation "test_foreign_table" DETAIL: This operation is not supported for foreign tables. +-- Check that Direct TOAST tables are verified without reporting false corruption +CREATE TABLE direct_toast_check (id int, val text) WITH (toast_flavour = 'direct'); +INSERT INTO direct_toast_check SELECT i, string_agg(md5(i::text), '') FROM generate_series(1, 10) i GROUP BY i; +SELECT * FROM verify_heapam('direct_toast_check', check_toast := true); + blkno | offnum | attnum | msg +-------+--------+--------+----- +(0 rows) + +DROP TABLE direct_toast_check; -- cleanup DROP TABLE heaptest; DROP TABLESPACE regress_test_stats_tblspc; diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/check_heap.sql index 1745bae634e..13bba6c04c8 100644 --- a/contrib/amcheck/sql/check_heap.sql +++ b/contrib/amcheck/sql/check_heap.sql @@ -138,6 +138,12 @@ SELECT * FROM verify_heapam('test_foreign_table', startblock := NULL, endblock := NULL); +-- Check that Direct TOAST tables are verified without reporting false corruption +CREATE TABLE direct_toast_check (id int, val text) WITH (toast_flavour = 'direct'); +INSERT INTO direct_toast_check SELECT i, string_agg(md5(i::text), '') FROM generate_series(1, 10) i GROUP BY i; +SELECT * FROM verify_heapam('direct_toast_check', check_toast := true); +DROP TABLE direct_toast_check; + -- cleanup DROP TABLE heaptest; DROP TABLESPACE regress_test_stats_tblspc; diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 20ff58aa782..ffd92453807 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -12,6 +12,7 @@ #include "access/detoast.h" #include "access/genam.h" +#include "access/heapam.h" #include "access/heaptoast.h" #include "access/multixact.h" #include "access/relation.h" @@ -27,6 +28,7 @@ #include "storage/lwlock.h" #include "storage/procarray.h" #include "storage/read_stream.h" +#include "utils/array.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/rel.h" @@ -75,7 +77,9 @@ typedef enum SkipPages */ typedef struct ToastedAttribute { + bool is_direct; varatt_external toast_pointer; + struct varatt_direct toast_pointer_direct; BlockNumber blkno; /* block in main table */ OffsetNumber offnum; /* offset in main table */ AttrNumber attnum; /* attribute in main table */ @@ -1768,6 +1772,84 @@ check_tuple_attribute(HeapCheckContext *ctx) return true; /* It is external, and we're looking at a page on disk */ + if (VARATT_IS_EXTERNAL_DIRECT(attr)) + { + struct varatt_direct toast_pointer; + + VARATT_EXTERNAL_GET_POINTER_DIRECT(toast_pointer, attr); + + /* Toasted attributes too large to be untoasted should never be stored */ + if (toast_pointer.va_rawsize > VARLENA_SIZE_LIMIT) + report_corruption(ctx, + psprintf("direct toast value rawsize %d exceeds limit %d", + toast_pointer.va_rawsize, + VARLENA_SIZE_LIMIT)); + + if (VARATT_DIRECT_IS_COMPRESSED(toast_pointer)) + { + ToastCompressionId cmid; + bool valid = false; + + /* Compressed attributes should have a valid compression method */ + cmid = VARATT_DIRECT_GET_COMPRESS_METHOD(toast_pointer); + switch (cmid) + { + /* List of all valid compression method IDs */ + case TOAST_PGLZ_COMPRESSION_ID: + case TOAST_LZ4_COMPRESSION_ID: + valid = true; + break; + + /* Recognized but invalid compression method ID */ + case TOAST_INVALID_COMPRESSION_ID: + break; + + /* Intentionally no default here */ + } + if (!valid) + report_corruption(ctx, + psprintf("direct toast value has invalid compression method id %d", + cmid)); + } + + /* The tuple header better claim to contain toasted values */ + if (!(infomask & HEAP_HASEXTERNAL)) + { + report_corruption(ctx, "direct toast value is external but tuple header flag HEAP_HASEXTERNAL not set"); + return true; + } + + /* The relation better have a toast table */ + if (!ctx->rel->rd_rel->reltoastrelid) + { + report_corruption(ctx, "direct toast value is external but relation has no toast relation"); + return true; + } + + /* If we were told to skip toast checking, then we're done. */ + if (ctx->toast_rel == NULL) + return true; + + /* + * If this tuple is eligible to be pruned, we cannot check the toast. + * Otherwise, we push a copy of the toast tuple so we can check it after + * releasing the main table buffer lock. + */ + if (!ctx->tuple_could_be_pruned) + { + ToastedAttribute *ta; + + ta = palloc0_object(ToastedAttribute); + ta->is_direct = true; + ta->toast_pointer_direct = toast_pointer; + ta->blkno = ctx->blkno; + ta->offnum = ctx->offnum; + ta->attnum = ctx->attnum; + ctx->toasted_attributes = lappend(ctx->toasted_attributes, ta); + } + + return true; + } /* * Must copy attr into toast_pointer for alignment considerations @@ -1841,7 +1923,7 @@ check_tuple_attribute(HeapCheckContext *ctx) ToastedAttribute *ta; ta = palloc0_object(ToastedAttribute); - + ta->is_direct = false; VARATT_EXTERNAL_GET_POINTER(ta->toast_pointer, attr); ta->blkno = ctx->blkno; ta->offnum = ctx->offnum; @@ -1852,6 +1934,99 @@ check_tuple_attribute(HeapCheckContext *ctx) return true; } +static void +check_toasted_attribute_direct_recursive(HeapCheckContext *ctx, ToastedAttribute *ta, + ItemPointer tid, uint32 *total_data_bytes) +{ + HeapTupleData tup; + Buffer buffer = InvalidBuffer; + bool isnull; + Pointer chunk; + int32 chunksize; + + tup.t_self = *tid; + if (!heap_fetch(ctx->toast_rel, get_toast_snapshot(), &tup, &buffer, false)) + { + report_toast_corruption(ctx, ta, + psprintf("direct toast chunk at TID (%u, %u) not found in toast table", + ItemPointerGetBlockNumber(tid), + ItemPointerGetOffsetNumber(tid))); + return; + } + + /* Check chunk_data (attribute 3) */ + chunk = DatumGetPointer(fastgetattr(&tup, 3, ctx->toast_rel->rd_att, &isnull)); + if (!isnull) + { + if (!VARATT_IS_EXTENDED(chunk)) + chunksize = VARSIZE(chunk) - VARHDRSZ; + else if (VARATT_IS_SHORT(chunk)) + chunksize = VARSIZE_SHORT(chunk) - VARHDRSZ_SHORT; + else + { + uint32 header = ((varattrib_4b *) chunk)->va_4byte.va_header; + + report_toast_corruption(ctx, ta, + psprintf("direct toast chunk at TID (%u, %u) has invalid varlena header %0x", + ItemPointerGetBlockNumber(tid), + ItemPointerGetOffsetNumber(tid), + header)); + ReleaseBuffer(buffer); + return; + } + + if (chunksize > TOAST_MAX_CHUNK_SIZE) + report_toast_corruption(ctx, ta, + psprintf("direct toast chunk at TID (%u, %u) has oversized chunk (%d bytes)", + ItemPointerGetBlockNumber(tid), + ItemPointerGetOffsetNumber(tid), + chunksize)); + + *total_data_bytes += chunksize; + } + + /* Check chunk_tids (attribute 4) if present */ + { + Datum tids_datum; + ArrayType *tids_arr; + Datum *tids_elems; + int num_tids; + int i; + + tids_datum = fastgetattr(&tup, 4, ctx->toast_rel->rd_att, &isnull); + if (!isnull) + { + tids_arr = DatumGetArrayTypeP(tids_datum); + deconstruct_array_builtin(tids_arr, TIDOID, &tids_elems, NULL, &num_tids); + for (i = 0; i < num_tids; i++) + { + ItemPointer child_tid = (ItemPointer) DatumGetPointer(tids_elems[i]); + + check_toasted_attribute_direct_recursive(ctx, ta, child_tid, total_data_bytes); + } + pfree(tids_elems); + } + } + + ReleaseBuffer(buffer); +} + +static void +check_toasted_attribute_direct(HeapCheckContext *ctx, ToastedAttribute *ta) +{ + uint32 extsize = VARATT_DIRECT_GET_EXTSIZE(ta->toast_pointer_direct); + uint32 total_data_bytes = 0; + + check_toasted_attribute_direct_recursive(ctx, ta, &ta->toast_pointer_direct.va_tid, &total_data_bytes); + + if (total_data_bytes != extsize) + { + report_toast_corruption(ctx, ta, + psprintf("direct toast value was expected to have %u bytes, but found %u bytes across chunks", + extsize, total_data_bytes)); + } +} + /* * For each attribute collected in ctx->toasted_attributes, look up the value * in the toast table and perform checks on it. This function should only be @@ -1869,6 +2044,12 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) int32 expected_chunk_seq = 0; int32 last_chunk_seq; + if (ta->is_direct) + { + check_toasted_attribute_direct(ctx, ta); + return; + } + extsize = VARATT_EXTERNAL_GET_EXTSIZE(ta->toast_pointer); last_chunk_seq = (extsize - 1) / TOAST_MAX_CHUNK_SIZE; -- 2.55.0.970.g62bdec98f9-goog