From 9bff4b2f17b9c3728b1a10a2d9fa4807c439b805 Mon Sep 17 00:00:00 2001 From: Nikhil Kumar Veldanda Date: Thu, 24 Sep 2026 00:30:27 -0700 Subject: [PATCH v16 1/5] 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. 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. Every caller checks VARATT_EXTINFO_IS_COMPRESSED() first, so this changes nothing for them. detoast.h now includes toast_compression.h, for ToastCompressionId. No behavior change. 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 | 2 +- src/backend/access/common/detoast.c | 4 ++-- src/backend/access/common/toast_compression.c | 2 +- src/include/access/detoast.h | 19 ++++++++++++++++++- src/include/varatt.h | 11 ++++------- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index ea1ad402ea2..d08c2995ff9 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1824,7 +1824,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 8223cc219c1..aa3794cf795 100644 --- a/src/backend/access/common/detoast.c +++ b/src/backend/access/common/detoast.c @@ -237,12 +237,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 c24cc38b6a0..d5307f086bc 100644 --- a/src/backend/access/common/toast_compression.c +++ b/src/backend/access/common/toast_compression.c @@ -296,7 +296,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..b203a1bfb7c 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,18 @@ 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); + + /* + * 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 (!VARATT_EXTINFO_IS_COMPRESSED(toast_ext_data->extinfo, + toast_ext_data->rawsize)) + 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