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

diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index b717b03b3d2c68e24e3a72b3439877b57b921ca7..9d900147a55f1c18ca9c1f843800fbef28b59b6b 100644 (file)
--- 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,