From 07d5bffe75d05b9c84803ea1ffaf5ce729f0e717 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 20 Mar 2026 14:16:06 +1300 Subject: [PATCH] Fix new tuple deforming code so it can support cstrings again In c456e3911, I mistakenly thought that the deformer code would never see cstrings and that I could use pg_assume() to have the compiler omit producing code for attlen == -2 attributes. That saves bloating the deforming code a bit with the extra check and strlen() call. While this is ok to do for tuples from the heap, it's not ok to do for MinimalTuples as those *can* contain cstrings and tts_minimal_getsomeattrs() implements deforming by inlining the (slightly misleadingly named) slot_deform_heap_tuple() code. To fix, add a new parameter to the slot_deform_heap_tuple() and have the callers define which code to inline. Because this new parameter is passed as a const, the compiler can choose to emit or not emit the cstring-related code based on the parameter's value. Author: David Rowley Reported-by: Tender Wang Discussion: https://postgr.es/m/CAHewXNmSK+gKziAt_WvQoMVWt3_LRVMmRYY9dAbMPMcpPV0QmA@mail.gmail.com --- src/backend/executor/execTuples.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index b717b03b3d2..9d900147a55 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -73,7 +73,7 @@ static TupleDesc ExecTypeFromTLInternal(List *targetList, bool skipjunk); static pg_attribute_always_inline void slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, - int reqnatts); + int reqnatts, bool support_cstring); static inline void tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, Buffer buffer, @@ -349,7 +349,7 @@ tts_heap_getsomeattrs(TupleTableSlot *slot, int natts) Assert(!TTS_EMPTY(slot)); - slot_deform_heap_tuple(slot, hslot->tuple, &hslot->off, natts); + slot_deform_heap_tuple(slot, hslot->tuple, &hslot->off, natts, false); } static Datum @@ -547,7 +547,7 @@ tts_minimal_getsomeattrs(TupleTableSlot *slot, int natts) Assert(!TTS_EMPTY(slot)); - slot_deform_heap_tuple(slot, mslot->tuple, &mslot->off, natts); + slot_deform_heap_tuple(slot, mslot->tuple, &mslot->off, natts, true); } /* @@ -754,7 +754,7 @@ tts_buffer_heap_getsomeattrs(TupleTableSlot *slot, int natts) Assert(!TTS_EMPTY(slot)); - slot_deform_heap_tuple(slot, bslot->base.tuple, &bslot->base.off, natts); + slot_deform_heap_tuple(slot, bslot->base.tuple, &bslot->base.off, natts, false); } static Datum @@ -1008,10 +1008,14 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, * * This is marked as always inline, so the different offp for different types * of slots gets optimized away. + * + * support_cstring should be passed as a const to allow the compiler only + * emit code during inlining for cstring deforming when it's required. + * cstrings can exist in MinimalTuples, but not in HeapTuples. */ static pg_attribute_always_inline void slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, - int reqnatts) + int reqnatts, bool support_cstring) { CompactAttribute *cattrs; CompactAttribute *cattr; @@ -1190,11 +1194,12 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, attlen = cattr->attlen; /* - * cstrings don't exist in heap tuples. Use pg_assume to instruct the - * compiler not to emit the cstring-related code in - * align_fetch_then_add(). + * Only emit the cstring-related code in align_fetch_then_add() when + * cstring support is needed. We assume support_cstring will be + * passed as a const to allow the compiler to eliminate this branch. */ - pg_assume(attlen > 0 || attlen == -1); + if (!support_cstring) + pg_assume(attlen > 0 || attlen == -1); /* align 'off', fetch the datum, and increment off beyond the datum */ values[attnum] = align_fetch_then_add(tp, @@ -1222,8 +1227,9 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, cattr = &cattrs[attnum]; attlen = cattr->attlen; - /* As above, we don't expect cstrings */ - pg_assume(attlen > 0 || attlen == -1); + /* As above, only emit cstring code when needed. */ + if (!support_cstring) + pg_assume(attlen > 0 || attlen == -1); /* align 'off', fetch the datum, and increment off beyond the datum */ values[attnum] = align_fetch_then_add(tp,