From 8160dd33825abfcae2a14e78ed85a4c8e94e0274 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 30 Jan 2026 23:18:45 +1300 Subject: [PATCH v8 4/4] Various experimental changes --- src/backend/access/common/tupdesc.c | 6 ++ src/backend/executor/execTuples.c | 63 ++++++------ src/include/access/tupmacs.h | 149 ++++++++++++++++++++++++++-- src/include/executor/tuptable.h | 4 +- 4 files changed, 183 insertions(+), 39 deletions(-) diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index 25364db630a..ca393af67c9 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -105,6 +105,12 @@ populate_compact_attribute_internal(Form_pg_attribute src, elog(ERROR, "invalid attalign value: %c", src->attalign); break; } + + /* Check for unsupported byval attlens */ + if (src->attbyval && src->attlen != sizeof(char) && + src->attlen != sizeof(int16) && src->attlen != sizeof(int32) && + src->attlen != sizeof(int64)) + elog(ERROR, "unsupported byval length: %d", src->attlen); } /* diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 36d0aaed2fb..5e20a05a830 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -1029,23 +1029,34 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, /* We can only fetch as many attributes as the tuple has. */ natts = Min(HeapTupleHeaderGetNatts(tup), natts); attnum = slot->tts_nvalid; + values = slot->tts_values; + isnull = slot->tts_isnull; firstNonCacheOffsetAttr = Min(tupleDesc->firstNonCachedOffAttr, natts); if (hasnulls) { + tp = (char *) tup + + MAXALIGN(offsetof(HeapTupleHeaderData, t_bits) + + BITMAPLEN(HeapTupleHeaderGetNatts(tup))); + Assert(tp == (char *) tup + tup->t_hoff); bp = tup->t_bits; firstNullAttr = first_null_attr(bp, natts); + populate_isnull_array(bp, natts, isnull); firstNonCacheOffsetAttr = Min(firstNonCacheOffsetAttr, firstNullAttr); } else { + uint64 *isnull64 = (uint64 *) isnull; + Size asize = (natts + 7) >> 3; + + tp = (char *) tup + MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)); bp = NULL; firstNullAttr = natts; - } - values = slot->tts_values; - isnull = slot->tts_isnull; - tp = (char *) tup + tup->t_hoff; + /* No nulls, set all isnull elements to false */ + for (int i = 0; i < asize; i++) + isnull64[i] = 0; + } /* * Handle the portion of the tuple that we have cached the offset for up @@ -1065,7 +1076,6 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, #endif do { - isnull[attnum] = false; cattr = TupleDescCompactAttr(tupleDesc, attnum); #ifdef USE_ASSERT_CHECKING @@ -1074,7 +1084,9 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, offcheck += cattr->attlen; #endif - values[attnum] = fetchatt(cattr, tp + cattr->attcacheoff); + values[attnum] = fetch_att_noerr(tp + cattr->attcacheoff, + cattr->attbyval, + cattr->attlen); } while (++attnum < firstNonCacheOffsetAttr); /* @@ -1101,19 +1113,14 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, */ for (; attnum < firstNullAttr; attnum++) { - isnull[attnum] = false; cattr = TupleDescCompactAttr(tupleDesc, attnum); - /* align the offset for this attribute */ - off = att_pointer_alignby(off, - cattr->attalignby, - cattr->attlen, - tp + off); - - values[attnum] = fetchatt(cattr, tp + off); - - /* move the offset beyond this attribute */ - off = att_addlength_pointer(off, cattr->attlen, tp + off); + /* align 'off', fetch the datum, and increment off beyond the datum */ + values[attnum] = align_fetch_then_add(tp, + &off, + cattr->attbyval, + cattr->attlen, + cattr->attalignby); } /* @@ -1122,26 +1129,20 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, */ for (; attnum < natts; attnum++) { - if (att_isnull(attnum, bp)) + if (isnull[attnum]) { values[attnum] = (Datum) 0; - isnull[attnum] = true; continue; } - isnull[attnum] = false; cattr = TupleDescCompactAttr(tupleDesc, attnum); - /* align the offset for this attribute */ - off = att_pointer_alignby(off, - cattr->attalignby, - cattr->attlen, - tp + off); - - values[attnum] = fetchatt(cattr, tp + off); - - /* move the offset beyond this attribute */ - off = att_addlength_pointer(off, cattr->attlen, tp + off); + /* align 'off', fetch the datum, and increment off beyond the datum */ + values[attnum] = align_fetch_then_add(tp, + &off, + cattr->attbyval, + cattr->attlen, + cattr->attalignby); } /* @@ -1452,7 +1453,7 @@ ExecSetSlotDescriptor(TupleTableSlot *slot, /* slot to change */ slot->tts_values = (Datum *) MemoryContextAlloc(slot->tts_mcxt, tupdesc->natts * sizeof(Datum)); slot->tts_isnull = (bool *) - MemoryContextAlloc(slot->tts_mcxt, tupdesc->natts * sizeof(bool)); + MemoryContextAlloc(slot->tts_mcxt, MAXALIGN(tupdesc->natts * sizeof(bool))); } /* -------------------------------- diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h index ce7a88df611..d53784899fb 100644 --- a/src/include/access/tupmacs.h +++ b/src/include/access/tupmacs.h @@ -16,7 +16,7 @@ #include "catalog/pg_type_d.h" /* for TYPALIGN macros */ #include "port/pg_bitutils.h" - +#include "varatt.h" /* * Check a tuple's null bitmap to determine whether the attribute is null. @@ -29,6 +29,49 @@ att_isnull(int ATT, const bits8 *BITS) return !(BITS[ATT >> 3] & (1 << (ATT & 0x07))); } +/* + * populate_isnull_array + * Transform a tuple's null bitmap into a boolean array. + * + * Caller must ensure that the isnull array is sized so it contains + * at least as many elements as there are bits in the 'bits' array. + * This is required because we always round 'natts' up to the next multiple + * of 8. + */ +static inline void +populate_isnull_array(const bits8 *bits, int natts, bool *isnull) +{ + int nbytes = (natts + 7) >> 3; + + /* + * Multiplying a NULL bitmap byte by this value results in the lowest bit + * in each byte being set the same as each bit of the bitmap. We perform + * this as 2 32-bit operations rather than a single 64-bit operation as + * multiplying by the required value to do this in 64-bits would result in + * overflowing a uint64 in some cases. + */ +#define SPREAD_BITS_MULTIPLIER_32 0x204081U + + for (int i = 0; i < nbytes; i++, isnull += 8) + { + uint64 isnull_8; + bits8 nullbyte = ~bits[i]; + + /* convert the lower 4 bits of null bitmap word into 32 bit int */ + isnull_8 = (nullbyte & 0xf) * SPREAD_BITS_MULTIPLIER_32; + + /* + * convert the upper 4 bits of null bitmap word into 32 bit int, shift + * into the upper 32 bit + */ + isnull_8 |= ((uint64) ((nullbyte >> 4) * SPREAD_BITS_MULTIPLIER_32)) << 32; + + /* mask out all other bits apart from the lowest bit of each byte */ + isnull_8 &= UINT64CONST(0x0101010101010101); + memcpy(isnull, &isnull_8, sizeof(uint64)); + } +} + #ifndef FRONTEND /* * Given an attbyval and an attlen from either a Form_pg_attribute or @@ -71,6 +114,100 @@ fetch_att(const void *T, bool attbyval, int attlen) return PointerGetDatum(T); } +/* + * Same, but no error checking for invalid attlens for byval types. This + * is safe to use when attlen comes from CompactAttribute as we validate the + * length when populating that struct. + */ +static inline Datum +fetch_att_noerr(const void *T, bool attbyval, int attlen) +{ + if (attbyval) + { + switch (attlen) + { + case sizeof(char): + return CharGetDatum(*((const char *) T)); + case sizeof(int16): + return Int16GetDatum(*((const int16 *) T)); + case sizeof(int32): + return Int32GetDatum(*((const int32 *) T)); + default: + Assert(attlen == sizeof(int64)); + return Int64GetDatum(*((const int64 *) T)); + } + } + else + return PointerGetDatum(T); +} + + +/* + * align_fetch_then_add + * Applies all the functionality of att_pointer_alignby(), fetch_att() + * and att_addlength_pointer() resulting in *off pointer to the perhaps + * unaligned number of bytes into 'tupptr', ready to deform the next + * attribute. + * + * tupptr: pointer to the beginning of the tuple, after the header and any + * NULL bitmask. + * off: offset in bytes for reading tuple data, possibly unaligned. + * attbyval, attlen, attalignby are values from CompactAttribute. + */ +static inline Datum +align_fetch_then_add(const char *tupptr, uint32 *off, bool attbyval, int attlen, + uint8 attalignby) +{ + Datum res; + + if (attlen > 0) + { + const char *offset_ptr; + + *off = TYPEALIGN(attalignby, *off); + offset_ptr = tupptr + *off; + *off += attlen; + if (attbyval) + { + switch (attlen) + { + case sizeof(char): + return CharGetDatum(*((const char *) offset_ptr)); + case sizeof(int16): + return Int16GetDatum(*((const int16 *) offset_ptr)); + case sizeof(int32): + return Int32GetDatum(*((const int32 *) offset_ptr)); + default: + + /* + * populate_compact_attribute_internal() should have + * checked + */ + Assert(attlen == sizeof(int64)); + return Int64GetDatum(*((const int64 *) offset_ptr)); + } + } + return PointerGetDatum(offset_ptr); + } + else if (attlen == -1) + { + if (!VARATT_IS_SHORT(tupptr + *off)) + *off = TYPEALIGN(attalignby, *off); + + res = PointerGetDatum(tupptr + *off); + *off += VARSIZE_ANY(DatumGetPointer(res)); + return res; + } + else + { + Assert(attlen == -2); + *off = TYPEALIGN(attalignby, *off); + res = PointerGetDatum(tupptr + *off); + *off += strlen(tupptr + *off) + 1; + return res; + } +} + #ifndef HAVE__BUILTIN_CTZ /* * For returning the 0-based position of the right-most 0 bit of a uint8, or 8 @@ -100,6 +237,9 @@ static const uint8 pg_rightmost_zero_pos[256] = { * first_null_attr * Inspect a NULL bitmask from a tuple and return the 0-based attnum of the * first NULL attribute. Returns natts if no NULLs were found. + * + * We expect that 'bits' contains at least one 0 bit somewhere in the mask, + * not necessarily < natts. */ static inline int first_null_attr(const bits8 *bits, int natts) @@ -133,12 +273,7 @@ first_null_attr(const bits8 *bits, int natts) res = bytenum << 3; #ifdef HAVE__BUILTIN_CTZ - /* - * Promote to 32-bit before doing bit-wise NOT. This means we'll convert - * 0xff into 0xffffff00 rather than 0x0, which is undefined with - * __builtin_ctz. That'll mean we correctly get 8 for 0xff - */ - res += __builtin_ctz(~(uint32) bits[bytenum]); + res += __builtin_ctz(~bits[bytenum]); #else res += pg_rightmost_zero_pos[bits[bytenum]]; #endif diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h index 363c5f33697..180fccc999f 100644 --- a/src/include/executor/tuptable.h +++ b/src/include/executor/tuptable.h @@ -116,7 +116,9 @@ typedef struct TupleTableSlot #define FIELDNO_TUPLETABLESLOT_VALUES 5 Datum *tts_values; /* current per-attribute values */ #define FIELDNO_TUPLETABLESLOT_ISNULL 6 - bool *tts_isnull; /* current per-attribute isnull flags */ + bool *tts_isnull; /* current per-attribute isnull flags. Array + * size must always be rounded up to the next + * 8 elements. */ MemoryContext tts_mcxt; /* slot itself is in this context */ ItemPointerData tts_tid; /* stored tuple's tid */ Oid tts_tableOid; /* table oid of tuple */ -- 2.51.0