Hi,

While reviewing another patch, I noticed this:
```
static void
heapam_tuple_complete_speculative(Relation relation, TupleTableSlot *slot,
                                                                  uint32 
specToken, bool succeeded)
{
        bool            shouldFree = true;
        HeapTuple       tuple = ExecFetchSlotHeapTuple(slot, true, 
&shouldFree); // <== tuple is not used

        /* adjust the tuple's state accordingly */
        if (succeeded)
                heap_finish_speculative(relation, &slot->tts_tid);
        else
                heap_abort_speculative(relation, &slot->tts_tid);

        if (shouldFree)
                pfree(tuple);
}
```

In this function, tuple is not used at all, so there seems to be no need to 
fetch it, and shouldFree is thus not needed either.

This appears to have been there since 5db6df0c011, where the function was 
introduced. It looks like a copy-pasto from the previous function, 
heapam_tuple_insert_speculative(), which does need to fetch and possibly free 
the tuple.

I tried simply removing ExecFetchSlotHeapTuple(), and "make check" still 
passes. But I may be missing something, so I’d like to confirm.

The attached patch just removes the unused tuple and shouldFree from this 
function. As touching the file, I also fixed a typo in the file header comment.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/




Attachment: v1-0001-heapam_handler-remove-unused-tuple-fetch-in-specu.patch
Description: Binary data

Reply via email to