From 3df46a2905efb3edb2bdf3f3361ad024f1238e7f Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Sun, 23 Aug 2026 20:13:47 +0000 Subject: [PATCH v4 5/9] 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 | 186 +++++++++++++++++++++++- 3 files changed, 199 insertions(+), 2 deletions(-) diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out index 435510eb252..b239f0418ed 100644 --- a/contrib/amcheck/expected/check_heap.out +++ b/contrib/amcheck/expected/check_heap.out @@ -286,6 +286,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 test_toast_oid; DROP TABLE test_toast_oid8; diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/check_heap.sql index 9ee25e01b23..da24a4683e2 100644 --- a/contrib/amcheck/sql/check_heap.sql +++ b/contrib/amcheck/sql/check_heap.sql @@ -168,6 +168,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 test_toast_oid; DROP TABLE test_toast_oid8; diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 27648894831..edf8e9d87b3 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/rel.h" #include "utils/tuplestore.h" @@ -74,6 +76,8 @@ typedef enum SkipPages */ typedef struct ToastedAttribute { + bool is_direct; + struct varatt_direct toast_pointer_direct; Oid8 va_valueid; /* value ID (works for both Oid and Oid8) */ uint32 va_extinfo; /* external size and compression method */ vartag_external tag; /* VARTAG_ONDISK_OID or VARTAG_ONDISK_OID8 */ @@ -1733,7 +1737,8 @@ check_tuple_attribute(HeapCheckContext *ctx) { uint8 va_tag = VARTAG_EXTERNAL(tp + ctx->offset); - if (va_tag != VARTAG_ONDISK_OID && va_tag != VARTAG_ONDISK_OID8) + if (va_tag != VARTAG_ONDISK_OID && va_tag != VARTAG_ONDISK_OID8 && + va_tag != VARTAG_DIRECT) { report_corruption(ctx, psprintf("toasted attribute has unexpected TOAST tag %u", @@ -1776,6 +1781,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 a decoded pointer for alignment considerations */ toast_external_info_get(attr, &toast_ext_data); @@ -1851,7 +1934,7 @@ check_tuple_attribute(HeapCheckContext *ctx) ToastedAttribute *ta; ta = palloc0_object(ToastedAttribute); - + ta->is_direct = false; /* The pointer has already been decoded above, just reuse it */ ta->tag = va_tag_value; ta->va_valueid = toast_pointer_valueid; @@ -1865,6 +1948,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(TupleDescAttr(ctx->toast_rel->rd_att, 0)->atttypid)) + 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 @@ -1886,6 +2062,12 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) Oid toast_typid; vartag_external expected_tag; + if (ta->is_direct) + { + check_toasted_attribute_direct(ctx, ta); + return; + } + toast_valueid = ta->va_valueid; extsize = VARATT_EXTINFO_GET_EXTSIZE(ta->va_extinfo); -- 2.55.0.1082.g2b9226bbc0-goog