From b8230f03fa70f470b0b3f50f31be6ea8940bb835 Mon Sep 17 00:00:00 2001 From: Nikhil Kumar Veldanda Date: Thu, 24 Sep 2026 00:30:27 -0700 Subject: [PATCH v17 1/5] Decode TOAST pointers fully in toast_external_info_get() toast_external_data holds the decoded contents of an on-disk TOAST pointer, but it kept the raw va_extinfo word, and each caller derived the saved size, whether the value is compressed, and the compression method from it with VARATT_EXTINFO_GET_EXTSIZE(), VARATT_EXTINFO_IS_COMPRESSED() and VARATT_EXTINFO_GET_COMPRESS_METHOD(). Decode all three once instead, in toast_external_info_get(), into new extsize, is_compressed and compress_method fields, and remove extinfo from the struct. detoast.c, toast_compression.c, reorderbuffer.c and amcheck now read the fields; amcheck's ToastedAttribute keeps the saved size instead of the raw word. This leaves toast_external_info_get() as the only place that knows how these are encoded in a TOAST pointer, which will matter once the encoding of the method is not the same for all methods. With no other caller left, remove VARATT_EXTINFO_GET_COMPRESS_METHOD(): its existence would only invite code to bypass toast_external_data. The writer side in toast_save_datum() still builds va_extinfo itself. For a value that is not compressed, compress_method is TOAST_INVALID_COMPRESSION_ID rather than whatever the method bits hold, so that the struct says by itself whether there is a method. detoast.h now includes toast_compression.h, for ToastCompressionId. No behavior change. Suggested-by: Michael Paquier Suggested-by: wenhui qiu Suggested-by: Dharin Shah Reviewed-by: Michael Paquier Reviewed-by: Japin Li Reviewed-by: Hannu Krosing Discussion: https://postgr.es/m/CAFAfj_GGY4W81XjtYLvYOhqPtvfZbBNj=YRS6Mq_0SA9AqEwoQ@mail.gmail.com Discussion: https://postgr.es/m/CAOj6k6dEVi0NvLjMLDhyrJS_n_NZO5D_OU89AO1u53u6NCDDwQ@mail.gmail.com --- contrib/amcheck/verify_heapam.c | 12 ++++---- src/backend/access/common/detoast.c | 18 +++++------ src/backend/access/common/toast_compression.c | 3 +- .../replication/logical/reorderbuffer.c | 4 +-- src/include/access/detoast.h | 30 +++++++++++++++++-- src/include/varatt.h | 11 +++---- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index ea1ad402ea2..289a3764e95 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -76,7 +76,7 @@ typedef enum SkipPages typedef struct ToastedAttribute { Oid8 va_valueid; /* value ID (works for both Oid and Oid8) */ - uint32 va_extinfo; /* external size and compression method */ + int32 extsize; /* external saved size */ vartag_external tag; /* VARTAG_ONDISK_OID or VARTAG_ONDISK_OID8 */ BlockNumber blkno; /* block in main table */ OffsetNumber offnum; /* offset in main table */ @@ -1697,7 +1697,6 @@ check_tuple_attribute(HeapCheckContext *ctx) uint16 infomask; Oid8 toast_pointer_valueid; int32 va_rawsize; - uint32 va_extinfo; CompactAttribute *thisatt; vartag_external va_tag_value; toast_external_data toast_ext_data; @@ -1808,7 +1807,6 @@ check_tuple_attribute(HeapCheckContext *ctx) va_tag_value = toast_ext_data.tag; toast_pointer_valueid = toast_ext_data.valueid; va_rawsize = toast_ext_data.rawsize; - va_extinfo = toast_ext_data.extinfo; /* Toasted attributes too large to be untoasted should never be stored */ if (va_rawsize > VARLENA_SIZE_LIMIT) @@ -1818,13 +1816,13 @@ check_tuple_attribute(HeapCheckContext *ctx) va_rawsize, VARLENA_SIZE_LIMIT)); - if (VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize)) + if (toast_ext_data.is_compressed) { ToastCompressionId cmid; 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 */ @@ -1881,7 +1879,7 @@ check_tuple_attribute(HeapCheckContext *ctx) /* The pointer has already been decoded above, just reuse it */ ta->tag = va_tag_value; ta->va_valueid = toast_pointer_valueid; - ta->va_extinfo = va_extinfo; + ta->extsize = toast_ext_data.extsize; ta->blkno = ctx->blkno; ta->offnum = ctx->offnum; ta->attnum = ctx->attnum; @@ -1913,7 +1911,7 @@ check_toasted_attribute(HeapCheckContext *ctx, ToastedAttribute *ta) vartag_external expected_tag; toast_valueid = ta->va_valueid; - extsize = VARATT_EXTINFO_GET_EXTSIZE(ta->va_extinfo); + extsize = ta->extsize; /* * Take the chunk_id type from the TOAST table's own definition, not from diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c index 8223cc219c1..a07703ab9cc 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -237,13 +237,13 @@ 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); - is_compressed = VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize); + extsize = toast_ext_data.extsize; + compress_method = toast_ext_data.compress_method; + is_compressed = toast_ext_data.is_compressed; /* fast path for non-compressed external datums */ if (!is_compressed) @@ -368,13 +368,13 @@ toast_fetch_datum(varlena *attr) elog(ERROR, "toast_fetch_datum shouldn't be called for non-ondisk datums"); toast_external_info_get(attr, &toast_ext_data); - attrsize = VARATT_EXTINFO_GET_EXTSIZE(toast_ext_data.extinfo); + attrsize = toast_ext_data.extsize; toastrelid = toast_ext_data.toastrelid; valueid = toast_ext_data.valueid; result = (varlena *) palloc(attrsize + VARHDRSZ); - if (VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize)) + if (toast_ext_data.is_compressed) SET_VARSIZE_COMPRESSED(result, attrsize + VARHDRSZ); else SET_VARSIZE(result, attrsize + VARHDRSZ); @@ -425,10 +425,10 @@ toast_fetch_datum_slice(varlena *attr, int32 sliceoffset, elog(ERROR, "toast_fetch_datum_slice shouldn't be called for non-ondisk datums"); toast_external_info_get(attr, &toast_ext_data); - attrsize = VARATT_EXTINFO_GET_EXTSIZE(toast_ext_data.extinfo); + attrsize = toast_ext_data.extsize; toastrelid = toast_ext_data.toastrelid; valueid = toast_ext_data.valueid; - is_compressed = VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize); + is_compressed = toast_ext_data.is_compressed; /* * It's nonsense to fetch slices of a compressed datum unless when it's a @@ -634,7 +634,7 @@ toast_datum_size(Datum value) toast_external_data toast_ext_data; toast_external_info_get(attr, &toast_ext_data); - result = VARATT_EXTINFO_GET_EXTSIZE(toast_ext_data.extinfo); + result = toast_ext_data.extsize; } else if (VARATT_IS_EXTERNAL_INDIRECT(attr)) { diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c index c24cc38b6a0..8250cf85879 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -295,8 +295,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/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 46843985317..acd3c9a80e8 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -5350,10 +5350,10 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn, VARSIZE(chunk) - VARHDRSZ); data_done += VARSIZE(chunk) - VARHDRSZ; } - Assert(data_done == VARATT_EXTINFO_GET_EXTSIZE(toast_ext_data.extinfo)); + Assert(data_done == toast_ext_data.extsize); /* make sure its marked as compressed or not */ - if (VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data.extinfo, toast_ext_data.rawsize)) + if (toast_ext_data.is_compressed) SET_VARSIZE_COMPRESSED(reconstructed, data_done + VARHDRSZ); else SET_VARSIZE(reconstructed, data_done + VARHDRSZ); diff --git a/src/include/access/detoast.h b/src/include/access/detoast.h index 93b7a253760..fe3444c6739 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,18 @@ do { \ /* * Decoded contents of an on-disk external TOAST pointer. + * + * Callers use these fields rather than decoding va_extinfo themselves. + * compress_method is TOAST_INVALID_COMPRESSION_ID when the value is not + * compressed. */ typedef struct toast_external_data { vartag_external tag; /* VARTAG_ONDISK_* */ int32 rawsize; /* original data size (includes header) */ - uint32 extinfo; /* saved size + compression method */ + int32 extsize; /* external saved size (without header) */ + bool is_compressed; /* is the saved data compressed? */ + 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; @@ -56,6 +63,8 @@ typedef struct toast_external_data static inline void toast_external_info_get(const struct varlena *attr, toast_external_data *toast_ext_data) { + uint32 extinfo; + Assert(VARATT_IS_EXTERNAL_ONDISK(attr)); toast_ext_data->tag = VARTAG_EXTERNAL(attr); @@ -65,7 +74,7 @@ toast_external_info_get(const struct varlena *attr, toast_external_data *toast_e VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); toast_ext_data->rawsize = toast_pointer.va_rawsize; - toast_ext_data->extinfo = toast_pointer.va_extinfo; + extinfo = toast_pointer.va_extinfo; toast_ext_data->valueid = VARATT_EXTERNAL_OID8_GET_VALUEID(toast_pointer); toast_ext_data->toastrelid = toast_pointer.va_toastrelid; } @@ -75,10 +84,25 @@ toast_external_info_get(const struct varlena *attr, toast_external_data *toast_e VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr); toast_ext_data->rawsize = toast_pointer.va_rawsize; - toast_ext_data->extinfo = toast_pointer.va_extinfo; + extinfo = toast_pointer.va_extinfo; toast_ext_data->valueid = toast_pointer.va_valueid; toast_ext_data->toastrelid = toast_pointer.va_toastrelid; } + + toast_ext_data->extsize = VARATT_EXTINFO_GET_EXTSIZE(extinfo); + toast_ext_data->is_compressed = + VARATT_EXTINFO_IS_COMPRESSED(extinfo, toast_ext_data->rawsize); + + /* + * An external datum that is not compressed has no method; report it as + * the invalid id rather than leaving the raw method bits (which read as + * pglz) in place, so toast_external_data is self-describing. + */ + if (toast_ext_data->is_compressed) + toast_ext_data->compress_method = (ToastCompressionId) + (extinfo >> VARLENA_EXTSIZE_BITS); + else + toast_ext_data->compress_method = TOAST_INVALID_COMPRESSION_ID; } /* ---------- diff --git a/src/include/varatt.h b/src/include/varatt.h index c22575a3624..602f45fa627 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -570,7 +570,10 @@ VARDATA_COMPRESSED_SET_TCINFO(void *PTR, uint32 extsize, uint32 cmethod) } /* - * Same for external Datums, saved into a va_extinfo. + * Same for the saved size of external Datums, stored in va_extinfo. + * + * There is deliberately no equivalent for the compression method: decode the + * pointer with toast_external_info_get() and use its compress_method field. */ static inline Size VARATT_EXTINFO_GET_EXTSIZE(uint32 extinfo) @@ -578,12 +581,6 @@ VARATT_EXTINFO_GET_EXTSIZE(uint32 extinfo) return extinfo & VARLENA_EXTSIZE_MASK; } -static inline uint32 -VARATT_EXTINFO_GET_COMPRESS_METHOD(uint32 extinfo) -{ - return extinfo >> VARLENA_EXTSIZE_BITS; -} - /* * Testing whether an externally-stored value is compressed requires comparing * the saved size stored in va_extinfo (the actual length of the external data) -- 2.50.1