Skip to content

Commit

Permalink
PG16: Fix concurrent update issues with MERGE.
Browse files Browse the repository at this point in the history 
PG16 commit postgres/postgres@9321c79c fixes an issue with concurrent
update issues in MERGE. This patch adapts that fix into the MERGE
support for hypertables as well.

postgres/postgres@9321c79c
  • Loading branch information
@lkshminarayanan
lkshminarayanan committed Aug 31, 2023
1 parent e4facda commit caada43
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 45 deletions.
86 changes: 67 additions & 19 deletions src/compat/compat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,12 @@ pg_strtoint64(const char *str)
} while (0)
#endif

/*
* PG15 updated the signatures of ExecARUpdateTriggers and ExecARDeleteTriggers while
* fixing foreign key handling during cross-partition updates
*
* https://github.com/postgres/postgres/commit/ba9a7e39217
*/
#if PG15_LT
#define ExecARUpdateTriggersCompat(estate, \
resultRelInfo, \
Expand All @@ -722,6 +728,13 @@ pg_strtoint64(const char *str)
inewslot, \
recheckIndexes, \
transtition_capture)
#define ExecARDeleteTriggersCompat(estate, \
resultRelInfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update) \
ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple, ar_delete_trig_tcs)
#else
#define ExecARUpdateTriggersCompat(estate, \
resultRelInfo, \
Expand All @@ -743,49 +756,84 @@ pg_strtoint64(const char *str)
recheckIndexes, \
transtition_capture, \
is_crosspart_update)
#define ExecARDeleteTriggersCompat(estate, \
resultRelInfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update) \
ExecARDeleteTriggers(estate, \
resultRelInfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update)
#endif

/*
* PG15 adds new argument TM_FailureData to ExecBRUpdateTriggers
* as a part of adding support for Merge
* https://github.com/postgres/postgres/commit/9321c79c
*
* PG16 adds TMResult argument to ExecBRUpdateTriggers
* https://github.com/postgres/postgres/commit/7103ebb7
*/
#if PG15_LT
#define ExecBRUpdateTriggersCompat(estate, \
epqstate, \
resultRelInfo, \
tupleid, \
oldtuple, \
slot, \
result, \
tmfdp) \
ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, tupleid, oldtuple, slot)
#else
#elif PG16_LT
#define ExecBRUpdateTriggersCompat(estate, \
epqstate, \
resultRelInfo, \
tupleid, \
oldtuple, \
slot, \
result, \
tmfdp) \
ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, tupleid, oldtuple, slot, tmfdp)
#endif

#if PG15_LT
#define ExecARDeleteTriggersCompat(estate, \
#else
#define ExecBRUpdateTriggersCompat(estate, \
epqstate, \
resultRelInfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update) \
ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple, ar_delete_trig_tcs)
slot, \
result, \
tmfdp) \
ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, tupleid, oldtuple, slot, result, tmfdp)
#endif

/*
* PG16 adds TMResult argument to ExecBRDeleteTriggers
* https://github.com/postgres/postgres/commit/9321c79c
*/
#if PG16_LT
#define ExecBRDeleteTriggersCompat(estate, \
epqstate, \
relinfo, \
tupleid, \
fdw_trigtuple, \
epqslot, \
tmresult, \
tmfd) \
ExecBRDeleteTriggers(estate, epqstate, relinfo, tupleid, fdw_trigtuple, epqslot)
#else
#define ExecARDeleteTriggersCompat(estate, \
resultRelInfo, \
#define ExecBRDeleteTriggersCompat(estate, \
epqstate, \
relinfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update) \
ExecARDeleteTriggers(estate, \
resultRelInfo, \
tupleid, \
oldtuple, \
ar_delete_trig_tcs, \
is_crosspart_update)
fdw_trigtuple, \
epqslot, \
tmresult, \
tmfd) \
ExecBRDeleteTriggers(estate, epqstate, relinfo, tupleid, fdw_trigtuple, epqslot, tmresult, tmfd)
#endif

#if (PG13 && PG_VERSION_NUM < 130010) || (PG14 && PG_VERSION_NUM < 140007)
Expand Down
63 changes: 47 additions & 16 deletions src/import/ht_hypertable_modify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,14 @@
* otherwise, return true.
*/
bool
ht_ExecUpdatePrologue(ModifyTableContext * context, ResultRelInfo * resultRelInfo, ItemPointer tupleid,
HeapTuple oldtuple, TupleTableSlot * slot)
ht_ExecUpdatePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot *slot,
TM_Result *result)
{
Relation resultRelationDesc = resultRelInfo->ri_RelationDesc;
Relation resultRelationDesc = resultRelInfo->ri_RelationDesc;

if (result != NULL)
*result = TM_Ok;

ExecMaterializeSlot(slot);

Expand All @@ -49,6 +53,7 @@ ht_ExecUpdatePrologue(ModifyTableContext * context, ResultRelInfo * resultRelInf
tupleid,
oldtuple,
slot,
result,
&context->tmfd);

return true;
Expand Down Expand Up @@ -193,16 +198,23 @@ ht_ExecUpdateEpilogue(ModifyTableContext * context, UpdateContext * updateCxt,
ModifyTableState *mtstate = context->mtstate;

/* insert index entries for tuple if necessary */

if (resultRelInfo->ri_NumIndices > 0 && updateCxt->updateIndexes)
bool onlySummarizing = false;
#if PG16_LT
bool updateIndexes = updateCxt->updateIndexes;
(void) onlySummarizing; /* onlySummarizing is unused in versions < PG16 */
#else
bool updateIndexes = (updateCxt->updateIndexes != TU_None);
onlySummarizing = (updateCxt->updateIndexes == TU_Summarizing);
#endif
if (resultRelInfo->ri_NumIndices > 0 && updateIndexes)
recheckIndexes = ExecInsertIndexTuplesCompat(resultRelInfo,
slot,
context->estate,
true,
false,
NULL,
NIL,
false);
onlySummarizing);

/* AFTER ROW UPDATE Triggers */
ExecARUpdateTriggersCompat(context->estate,
Expand Down Expand Up @@ -240,17 +252,20 @@ ht_ExecUpdateEpilogue(ModifyTableContext * context, UpdateContext * updateCxt,
* the delete a no-op; otherwise, return true.
*/
bool
ht_ExecDeletePrologue(ModifyTableContext * context, ResultRelInfo * resultRelInfo, ItemPointer tupleid,
HeapTuple oldtuple, TupleTableSlot * *epqreturnslot)
ht_ExecDeletePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot **epqreturnslot,
TM_Result *result)
{
/* BEFORE ROW DELETE triggers */
if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_delete_before_row)
return ExecBRDeleteTriggers(context->estate,
context->epqstate,
resultRelInfo,
tupleid,
oldtuple,
epqreturnslot);
return ExecBRDeleteTriggersCompat(context->estate,
context->epqstate,
resultRelInfo,
tupleid,
oldtuple,
epqreturnslot,
result,
&context->tmfd);

return true;
}
Expand Down Expand Up @@ -453,8 +468,16 @@ lmerge_matched:;
context->GetUpdateNewTuple = mergeGetUpdateNewTuple;
context->cpUpdateRetrySlot = NULL;

if (!ht_ExecUpdatePrologue(context, resultRelInfo, tupleid, NULL, newslot)) {
if (!ht_ExecUpdatePrologue(context, resultRelInfo, tupleid, NULL, newslot, &result))
{
#if PG16_LT
result = TM_Ok;
#else
if (result == TM_Ok)
return true; /* "do nothing" */

/* if not TM_OK, it is concurrent update/delete */
#endif
break;
}
ht_ExecUpdatePrepareSlot(resultRelInfo, newslot, context->estate);
Expand All @@ -480,8 +503,16 @@ lmerge_matched:;

case CMD_DELETE:
context->relaction = relaction;
if (!ht_ExecDeletePrologue(context, resultRelInfo, tupleid, NULL, NULL)) {
if (!ht_ExecDeletePrologue(context, resultRelInfo, tupleid, NULL, NULL, &result))
{
#if PG16_LT
result = TM_Ok;
#else
if (result == TM_Ok)
return true; /* "do nothing" */

/* if not TM_OK, it is concurrent update/delete */
#endif
break;
}
result = ht_ExecDeleteAct(context, resultRelInfo, tupleid, false);
Expand Down
22 changes: 14 additions & 8 deletions src/import/ht_hypertable_modify.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,21 @@ typedef struct ModifyTableContext {
/*
* Context struct containing output data specific to UPDATE operations.
*/
typedef struct UpdateContext {
bool updated; /* did UPDATE actually occur? */
bool updateIndexes; /* index update required? */
bool crossPartUpdate; /* was it a cross-partition
* update? */
} UpdateContext;
typedef struct UpdateContext
{
bool updated; /* did UPDATE actually occur? */
#if PG16_LT
bool updateIndexes; /* index update required? */
#else
TU_UpdateIndexes updateIndexes; /* result codes */
#endif

bool crossPartUpdate; /* was it a cross-partition
* update? */
} UpdateContext;

bool ht_ExecUpdatePrologue(ModifyTableContext * context, ResultRelInfo * resultRelInfo,
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot * slot);
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot * slot, TM_Result *result);
void ht_ExecUpdatePrepareSlot(ResultRelInfo * resultRelInfo, TupleTableSlot * slot, EState * estate);
TM_Result ht_ExecUpdateAct(ModifyTableContext * context, ResultRelInfo * resultRelInfo,
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot * slot,
Expand All @@ -90,7 +96,7 @@ void ht_ExecUpdateEpilogue(ModifyTableContext * context, UpdateContext * update
TupleTableSlot * slot, List * recheckIndexes);

bool ht_ExecDeletePrologue(ModifyTableContext * context, ResultRelInfo * resultRelInfo,
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot * *epqreturnslot);
ItemPointer tupleid, HeapTuple oldtuple, TupleTableSlot * *epqreturnslot, TM_Result *result);
TM_Result ht_ExecDeleteAct(ModifyTableContext * context, ResultRelInfo * resultRelInfo,
ItemPointer tupleid, bool changingPart);
void ht_ExecDeleteEpilogue(ModifyTableContext * context, ResultRelInfo * resultRelInfo,
Expand Down
4 changes: 2 additions & 2 deletions src/nodes/hypertable_modify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,7 @@ ExecUpdate(ModifyTableContext *context, ResultRelInfo *resultRelInfo, ItemPointe
* Prepare for the update. This includes BEFORE ROW triggers, so we're
* done if it says we are.
*/
if (!ht_ExecUpdatePrologue(context, resultRelInfo, tupleid, oldtuple, slot))
if (!ht_ExecUpdatePrologue(context, resultRelInfo, tupleid, oldtuple, slot, NULL))
return NULL;

/* INSTEAD OF ROW UPDATE Triggers */
Expand Down Expand Up @@ -2539,7 +2539,7 @@ ExecDelete(ModifyTableContext *context, ResultRelInfo *resultRelInfo, ItemPointe
* Prepare for the delete. This includes BEFORE ROW triggers, so we're
* done if it says we are.
*/
if (!ht_ExecDeletePrologue(context, resultRelInfo, tupleid, oldtuple, epqreturnslot))
if (!ht_ExecDeletePrologue(context, resultRelInfo, tupleid, oldtuple, epqreturnslot, NULL))
return NULL;

/* INSTEAD OF ROW DELETE Triggers */
Expand Down

0 comments on commit caada43

Please sign in to comment.