Re: Table AM callback table_complete_speculative()'s succeeded argument is reversed

Hi,


On May 14, 2019 4:29:01 AM PDT, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>The 'succeeded' argument seems backwards here:
>
>> static void
>> heapam_tuple_complete_speculative(Relation relation, TupleTableSlot
>*slot,
>>                                   uint32 spekToken, bool succeeded)
>> {
>>     bool        shouldFree = true;
>>     HeapTuple    tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree);
>>
>>     /* 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);
>> }
>
>According to the comments, if "succeeded = true", the insertion is
>completed, and otherwise it's killed. It works, because the only caller
>
>is also passing the argument wrong.

Thanks for finding.


>Barring objections, I'll push the attached patch to fix that.

Please hold off - your colleagues found this before, and I worked on getting test coverage for the code. It's scheduled
forcommit together today. Unfortunately nobody looked at the test much... 

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.