Handle table_complete_speculative's succeeded argument as documented.

For some reason both callsite and the implementation for heapam had
the meaning inverted (i.e. succeeded == true was passed in case of
conflict). That's confusing.

I (Andres) briefly pondered whether it'd be better to rename
table_complete_speculative's argument to 'bool specConflict' or such,
but decided not to. The 'complete' in the function name for me makes
`succeeded` sound a bit better.

Reported-By: Ashwin Agrawal, Melanie Plageman, Heikki Linnakangas
Discussion:
   https://postgr.es/m/CALfoeitk7-TACwYv3hCw45FNPjkA86RfXg4iQ5kAOPhR+F1Y4w@mail.gmail.com
   https://postgr.es/m/97673451-339f-b21e-a781-998d06b1067c@iki.fi

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index bc47856ad539ac7dfeb88fcc7a40f9e219143cba..00505ec3f4d8d7447a5ca6620814512a64b5fe75 100644 (file)
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -282,7 +282,7 @@ heapam_tuple_complete_speculative(Relation relation, TupleTableSlot *slot,
        HeapTuple       tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree);
 
        /* adjust the tuple's state accordingly */
-       if (!succeeded)
+       if (succeeded)
                heap_finish_speculative(relation, &slot->tts_tid);
        else
                heap_abort_speculative(relation, &slot->tts_tid);

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 444c0c057463b2cda33f4bd999add7b4ebca41b6..d545bbce8a29fb117417905e46969cadac454d23 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -556,7 +556,7 @@ ExecInsert(ModifyTableState *mtstate,
 
                        /* adjust the tuple's state accordingly */
                        table_complete_speculative(resultRelationDesc, slot,
-                                                                          specToken, specConflict);
+                                                                          specToken, !specConflict);
 
                        /*
                         * Wake up anyone waiting for our decision.  They will re-check