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

Hi,

On 2019-05-14 07:06:34 -0700, Andres Freund wrote:
> 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
scheduledfor commit together today. Unfortunately nobody looked at the test much...
 

\
And pushed, as https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=aa4b8c61d2cd57b53be03defb04d59b232a0e150
with the part that wasn't covered by tests now covered by
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=08e2edc0767ab6e619970f165cb34d4673105f23

Greetings,

Andres Freund