From 2901e859ad50f62ca260c60f276f556343a07895 Mon Sep 17 00:00:00 2001 From: Nikhil Kumar Veldanda Date: Thu, 24 Sep 2026 00:30:27 -0700 Subject: [PATCH v4 2/4] Decode the compression method in toast_external_info_get() The compression method of an external value was fetched by each caller from the raw method bits of va_extinfo, with VARATT_EXTINFO_GET_COMPRESS_METHOD(). Decode it once instead, 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 leaves toast_external_info_get() as the only place that knows how the method is encoded in a TOAST pointer, so remove VARATT_EXTINFO_GET_COMPRESS_METHOD(): with no other caller left, its existence would only invite code to bypass toast_external_data. This will matter once the encoding is not the same for all methods. detoast.h now includes toast_compression.h, for ToastCompressionId. No behavior change. --- contrib/amcheck/verify_heapam.c | 2 +- src/backend/access/common/detoast.c | 4 ++-- src/backend/access/common/toast_compression.c | 2 +- src/include/access/detoast.h | 10 +++++++++- src/include/varatt.h | 11 ++++------- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 27efe3e20f9..cb7938aed6c 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1817,7 +1817,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 a9d4e3ae98e..1960e6adae3 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -228,12 +228,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 */ 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/include/access/detoast.h b/src/include/access/detoast.h index 93b7a253760..d8edcdf259b 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) + (toast_ext_data->extinfo >> VARLENA_EXTSIZE_BITS); } /* ---------- diff --git a/src/include/varatt.h b/src/include/varatt.h index 7951a946a87..57175b39b24 100644 --- a/src/include/varatt.h +++ b/src/include/varatt.h @@ -569,7 +569,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) @@ -577,12 +580,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.54.0 (Apple Git-157)