From d5155497a2bf619ab5a803d5a07f3a36024836be Mon Sep 17 00:00:00 2001 From: Nikhil Kumar Veldanda Date: Tue, 15 Sep 2026 22:50:11 -0700 Subject: [PATCH v2 1/3] Refactor TOAST compression header handling This is preparatory refactoring with no behavior change, to make the compressed varlena header and TOAST pointer formats easier to extend with further compression methods. toast_internals.h duplicated the layout of the compressed-in-line header as toast_compress_header, with its own TOAST_COMPRESS_* macros to read and write the tcinfo word. varatt.h already describes the same bytes as varattrib_4b.va_compressed and provides accessors for them. Remove the duplicate and write the header through a single inline function, toast_compress_set_size_and_method(), so that there is one definition of this layout. The compression method of an external value was fetched by callers directly from the raw method bits of va_extinfo. Instead, decode it once in toast_external_info_get() into a new compress_method field of toast_external_data, and make detoast_attr_slice(), toast_get_compression_id() and amcheck read it from there. This keeps the knowledge of how the method is encoded in the pointer in a single place. Also add VARTAG_IS_ONDISK() alongside VARTAG_IS_EXPANDED(), and use it in VARATT_IS_EXTERNAL_ONDISK() and amcheck, and factor the assembly of the on-disk TOAST pointer datum in toast_save_datum() into a helper, toast_pointer_build(). --- contrib/amcheck/verify_heapam.c | 4 +- src/backend/access/common/detoast.c | 10 ++--- src/backend/access/common/toast_compression.c | 2 +- src/backend/access/common/toast_internals.c | 36 +++++++++++++---- src/include/access/detoast.h | 10 ++++- src/include/access/toast_internals.h | 39 ++++++++----------- src/include/varatt.h | 14 ++++--- src/tools/pgindent/typedefs.list | 1 - 8 files changed, 71 insertions(+), 45 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 27648894831..b6876d0f394 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1733,7 +1733,7 @@ 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 (!VARTAG_IS_ONDISK(va_tag)) { report_corruption(ctx, psprintf("toasted attribute has unexpected TOAST tag %u", @@ -1798,7 +1798,7 @@ check_tuple_attribute(HeapCheckContext *ctx) bool valid = false; /* Compressed attributes should have a valid compression method */ - cmid = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo); + cmid = toast_ext_data.compress_method; switch (cmid) { /* List of all valid compression method IDs */ diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c index 06992de1ede..25140372238 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -227,12 +227,12 @@ detoast_attr_slice(varlena *attr, { toast_external_data toast_ext_data; int32 extsize; - uint32 compress_method; + ToastCompressionId compress_method; bool is_compressed; toast_external_info_get(attr, &toast_ext_data); extsize = VARATT_EXTINFO_GET_EXTSIZE(toast_ext_data.extinfo); - compress_method = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo); + compress_method = toast_ext_data.compress_method; is_compressed = VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize); /* fast path for non-compressed external datums */ @@ -489,7 +489,7 @@ toast_decompress_datum(varlena *attr) * Fetch the compression method id stored in the compression header and * decompress the data using the appropriate decompression routine. */ - cmid = TOAST_COMPRESS_METHOD(attr); + cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); switch (cmid) { case TOAST_PGLZ_COMPRESSION_ID: @@ -525,14 +525,14 @@ toast_decompress_datum_slice(varlena *attr, int32 slicelength) * have been seen to give wrong results if passed an output size that is * more than the data's true decompressed size. */ - if ((uint32) slicelength >= TOAST_COMPRESS_EXTSIZE(attr)) + if ((uint32) slicelength >= VARDATA_COMPRESSED_GET_EXTSIZE(attr)) return toast_decompress_datum(attr); /* * Fetch the compression method id stored in the compression header and * decompress the data slice using the appropriate decompression routine. */ - cmid = TOAST_COMPRESS_METHOD(attr); + cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); switch (cmid) { case TOAST_PGLZ_COMPRESSION_ID: diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index 09cdf880e4f..8bb871c9791 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -267,7 +267,7 @@ toast_get_compression_id(varlena *attr) toast_external_info_get(attr, &toast_ext_data); if (VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize)) - cmid = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo); + cmid = toast_ext_data.compress_method; } else if (VARATT_IS_COMPRESSED(attr)) cmid = VARDATA_COMPRESSED_GET_COMPRESS_METHOD(attr); diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c index a4e8a096254..f5e7e834921 100644 --- a/src/backend/access/common/toast_internals.c +++ b/src/backend/access/common/toast_internals.c @@ -28,6 +28,8 @@ static bool toastrel_valueid_exists(Relation toastrel, Oid8 valueid); static bool toastid_valueid_exists(Oid toastrelid, Oid8 valueid); +static varlena *toast_pointer_build(vartag_external tag, const void *fixed, + Size fixedsize); /* ---------- * toast_compress_datum - @@ -92,7 +94,7 @@ toast_compress_datum(Datum value, char cmethod) { /* successful compression */ Assert(cmid != TOAST_INVALID_COMPRESSION_ID); - TOAST_COMPRESS_SET_SIZE_AND_COMPRESS_METHOD(tmp, valsize, cmid); + toast_compress_set_size_and_method(tmp, valsize, cmid); return PointerGetDatum(tmp); } else @@ -390,9 +392,8 @@ toast_save_datum(Relation rel, Datum value, VARATT_EXTERNAL_OID8_SET_VALUEID(&toast_pointer, va_valueid); toast_pointer.va_toastrelid = va_toastrelid; - result = (varlena *) palloc(TOAST_OID8_POINTER_SIZE); - SET_VARTAG_EXTERNAL(result, VARTAG_ONDISK_OID8); - memcpy(VARDATA_EXTERNAL(result), &toast_pointer, sizeof(toast_pointer)); + result = toast_pointer_build(VARTAG_ONDISK_OID8, + &toast_pointer, sizeof(toast_pointer)); } else { @@ -403,14 +404,35 @@ toast_save_datum(Relation rel, Datum value, toast_pointer.va_valueid = (Oid) va_valueid; toast_pointer.va_toastrelid = va_toastrelid; - result = (varlena *) palloc(TOAST_OID_POINTER_SIZE); - SET_VARTAG_EXTERNAL(result, VARTAG_ONDISK_OID); - memcpy(VARDATA_EXTERNAL(result), &toast_pointer, sizeof(toast_pointer)); + result = toast_pointer_build(VARTAG_ONDISK_OID, + &toast_pointer, sizeof(toast_pointer)); } return PointerGetDatum(result); } +/* ---------- + * toast_pointer_build - + * + * Build an on-disk TOAST pointer datum of the given tag from its fixed + * part (a varatt_external_oid or varatt_external_oid8). + * ---------- + */ +static varlena * +toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize) +{ + varlena *result; + + Assert(VARTAG_IS_ONDISK(tag)); + Assert(VARTAG_SIZE(tag) == fixedsize); + + result = (varlena *) palloc(VARHDRSZ_EXTERNAL + VARTAG_SIZE(tag)); + SET_VARTAG_EXTERNAL(result, tag); + memcpy(VARDATA_EXTERNAL(result), fixed, fixedsize); + + return result; +} + /* ---------- * toast_valueid_scankey_init - * diff --git a/src/include/access/detoast.h b/src/include/access/detoast.h index 93b7a253760..9b9c253432c 100644 --- a/src/include/access/detoast.h +++ b/src/include/access/detoast.h @@ -12,6 +12,7 @@ #ifndef DETOAST_H #define DETOAST_H +#include "access/toast_compression.h" #include "varatt.h" /* @@ -40,12 +41,16 @@ do { \ /* * Decoded contents of an on-disk external TOAST pointer. + * + * compress_method is only meaningful if the value is compressed, that is if + * VARATT_EXTINFO_IS_COMPRESSED(extinfo, rawsize). */ typedef struct toast_external_data { vartag_external tag; /* VARTAG_ONDISK_* */ int32 rawsize; /* original data size (includes header) */ - uint32 extinfo; /* saved size + compression method */ + uint32 extinfo; /* saved size + compression method bits */ + ToastCompressionId compress_method; /* compression method ID */ Oid8 valueid; /* value ID (can be widened from Oid) */ Oid toastrelid; /* OID of the TOAST table containing it */ } toast_external_data; @@ -79,6 +84,9 @@ toast_external_info_get(const struct varlena *attr, toast_external_data *toast_e toast_ext_data->valueid = toast_pointer.va_valueid; toast_ext_data->toastrelid = toast_pointer.va_toastrelid; } + + toast_ext_data->compress_method = (ToastCompressionId) + VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data->extinfo); } /* ---------- diff --git a/src/include/access/toast_internals.h b/src/include/access/toast_internals.h index e03cc1204be..033bf45f0d1 100644 --- a/src/include/access/toast_internals.h +++ b/src/include/access/toast_internals.h @@ -17,34 +17,29 @@ #include "storage/lockdefs.h" #include "utils/relcache.h" #include "utils/snapshot.h" +#include "varatt.h" /* - * The information at the start of the compressed toast data. + * Fill in the header of a compressed-in-line datum: the original data size + * (excluding header) and the compression method. + * + * The compression routine must already have laid out the datum with + * VARHDRSZ_COMPRESSED bytes of header, since the compressed data starts + * right after it. The varlena length word is not touched here. */ -typedef struct toast_compress_header +static inline void +toast_compress_set_size_and_method(varlena *ptr, uint32 rawsize, + ToastCompressionId cmid) { - int32 vl_len_; /* varlena header (do not touch directly!) */ - uint32 tcinfo; /* 2 bits for compression method and 30 bits - * external size; see va_extinfo */ -} toast_compress_header; + varattrib_4b *va = (varattrib_4b *) ptr; -/* - * Utilities for manipulation of header information for compressed - * toast entries. - */ -#define TOAST_COMPRESS_EXTSIZE(ptr) \ - (((toast_compress_header *) (ptr))->tcinfo & VARLENA_EXTSIZE_MASK) -#define TOAST_COMPRESS_METHOD(ptr) \ - (((toast_compress_header *) (ptr))->tcinfo >> VARLENA_EXTSIZE_BITS) + Assert(rawsize > 0 && rawsize <= VARLENA_EXTSIZE_MASK); + Assert(cmid == TOAST_PGLZ_COMPRESSION_ID || + cmid == TOAST_LZ4_COMPRESSION_ID); -#define TOAST_COMPRESS_SET_SIZE_AND_COMPRESS_METHOD(ptr, len, cm_method) \ - do { \ - Assert((len) > 0 && (len) <= VARLENA_EXTSIZE_MASK); \ - Assert((cm_method) == TOAST_PGLZ_COMPRESSION_ID || \ - (cm_method) == TOAST_LZ4_COMPRESSION_ID); \ - ((toast_compress_header *) (ptr))->tcinfo = \ - (len) | ((uint32) (cm_method) << VARLENA_EXTSIZE_BITS); \ - } while (0) + va->va_compressed.va_tcinfo = + rawsize | ((uint32) cmid << VARLENA_EXTSIZE_BITS); +} extern Datum toast_compress_datum(Datum value, char cmethod); extern Oid toast_get_valid_index(Oid toastoid, LOCKMODE lock); diff --git a/src/include/varatt.h b/src/include/varatt.h index 77041f7673a..f6a1f5d60ae 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -141,6 +141,13 @@ VARTAG_IS_EXPANDED(vartag_external tag) return ((tag & ~1) == VARTAG_EXPANDED_RO); } +/* Is a TOAST pointer any of the on-disk kinds? */ +static inline bool +VARTAG_IS_ONDISK(vartag_external tag) +{ + return (tag == VARTAG_ONDISK_OID || tag == VARTAG_ONDISK_OID8); +} + /* Size of the data part of a "TOAST pointer" datum */ static inline Size VARTAG_SIZE(vartag_external tag) @@ -406,12 +413,7 @@ VARATT_IS_EXTERNAL(const void *PTR) static inline bool VARATT_IS_EXTERNAL_ONDISK(const void *PTR) { - vartag_external tag; - - if (!VARATT_IS_EXTERNAL(PTR)) - return false; - tag = VARTAG_EXTERNAL(PTR); - return (tag == VARTAG_ONDISK_OID || tag == VARTAG_ONDISK_OID8); + return VARATT_IS_EXTERNAL(PTR) && VARTAG_IS_ONDISK(VARTAG_EXTERNAL(PTR)); } /* Is varlena datum an indirect pointer? */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 0dc817cc2b8..b3992db9a59 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4356,7 +4356,6 @@ timeout_handler_proc timeout_params timerCA tlist_vinfo -toast_compress_header toast_external_data tokenize_error_callback_arg transferMode base-commit: 999ce9bcd80890c3d723e113bb0337c900f9f249 -- 2.54.0 (Apple Git-157)