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 <dgrowleyml@gmail.com>
Reported-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAHewXNmSK+gKziAt_WvQoMVWt3_LRVMmRYY9dAbMPMcpPV0QmA@mail.gmail.com
This commit is contained in:
David Rowley 2026-03-20 14:16:06 +13:00
parent 703fee3b25
commit 07d5bffe75

View file

@ -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,