Hi,

Back in b89e151054a0, the following macro has been introduced to
retrieve the varatt_external of an on-disk external TOAST Datum, stuff
now in detoast.h:
/*
 * Macro to fetch the possibly-unaligned contents of an EXTERNAL datum
 * into a local "struct varatt_external" toast pointer.  This should be
 * just a memcpy, but some versions of gcc seem to produce broken code
 * that assumes the datum contents are aligned.  Introducing an explicit
 * intermediate "varattrib_1b_e *" variable seems to fix it.
 */
#define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
do { \
        varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \
        Assert(VARATT_IS_EXTERNAL(attre)); \
        Assert(VARSIZE_EXTERNAL(attre) == sizeof(toast_pointer) + 
VARHDRSZ_EXTERNAL); \
        memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), 
sizeof(toast_pointer)); \
} while (0)

I vaguely recall that this has been mentioned during the unconference
session dedicated to TOAST, or perhaps not.  Anyway, I've just bumped
into that again while working on this area, and I am wondering if this
is relevant these days.

varattrib_1b_e should never be referenced directly in the code, so it
would be nice to clean up things like in the attached.  The CI is OK
with that, which is not the buildfarm but it's a start.

Comments or opinions?
--
Michael
From 3c460ef0c4fc3d18082a2d2b34bbb3f16f5c1f19 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 9 Jun 2025 17:11:37 +0900
Subject: [PATCH] Simplify gcc-related tweak in VARATT_EXTERNAL_GET_POINTER()

Introduced in b89e151054a0.
---
 src/include/access/detoast.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/include/access/detoast.h b/src/include/access/detoast.h
index e603a2276c38..7928f4d3012c 100644
--- a/src/include/access/detoast.h
+++ b/src/include/access/detoast.h
@@ -14,17 +14,13 @@
 
 /*
  * Macro to fetch the possibly-unaligned contents of an EXTERNAL datum
- * into a local "struct varatt_external" toast pointer.  This should be
- * just a memcpy, but some versions of gcc seem to produce broken code
- * that assumes the datum contents are aligned.  Introducing an explicit
- * intermediate "varattrib_1b_e *" variable seems to fix it.
+ * into a local "struct varatt_external" toast pointer.
  */
 #define VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr) \
 do { \
-	varattrib_1b_e *attre = (varattrib_1b_e *) (attr); \
-	Assert(VARATT_IS_EXTERNAL(attre)); \
-	Assert(VARSIZE_EXTERNAL(attre) == sizeof(toast_pointer) + VARHDRSZ_EXTERNAL); \
-	memcpy(&(toast_pointer), VARDATA_EXTERNAL(attre), sizeof(toast_pointer)); \
+	Assert(VARATT_IS_EXTERNAL(attr)); \
+	Assert(VARSIZE_EXTERNAL(attr) == sizeof(toast_pointer) + VARHDRSZ_EXTERNAL); \
+	memcpy(&(toast_pointer), VARDATA_EXTERNAL(attr), sizeof(toast_pointer)); \
 } while (0)
 
 /* Size of an EXTERNAL datum that contains a standard TOAST pointer */
-- 
2.49.0

Attachment: signature.asc
Description: PGP signature

Reply via email to