diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c new file mode 100644 index 66401f2..c2a49b9 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2943,7 +2943,8 @@ ExecBRUpdateTriggers(EState *estate, EPQ ItemPointer tupleid, HeapTuple fdw_trigtuple, TupleTableSlot *newslot, - TM_FailureData *tmfd) + TM_FailureData *tmfd, + MergeActionState *relaction) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, relinfo); @@ -2988,7 +2989,7 @@ ExecBRUpdateTriggers(EState *estate, EPQ TupleTableSlot *epqslot_clean; epqslot_clean = ExecGetUpdateNewTuple(relinfo, epqslot_candidate, - oldslot); + oldslot, relaction); if (newslot != epqslot_clean) ExecCopySlot(newslot, epqslot_clean); diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c new file mode 100644 index c484f5c..afc946b --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -486,7 +486,7 @@ ExecSimpleRelationUpdate(ResultRelInfo * resultRelInfo->ri_TrigDesc->trig_update_before_row) { if (!ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, - tid, NULL, slot, NULL)) + tid, NULL, slot, NULL, NULL)) skip_tuple = true; /* "do nothing" */ } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c new file mode 100644 index 6f44d71..7660c57 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -88,15 +88,6 @@ typedef struct ModifyTableContext */ TupleTableSlot *planSlot; - /* - * During EvalPlanQual, project and return the new version of the new - * tuple - */ - TupleTableSlot *(*GetUpdateNewTuple) (ResultRelInfo *resultRelInfo, - TupleTableSlot *epqslot, - TupleTableSlot *oldSlot, - MergeActionState *relaction); - /* MERGE specific */ MergeActionState *relaction; /* MERGE action in progress */ @@ -162,10 +153,6 @@ static TupleTableSlot *ExecPrepareTupleR ResultRelInfo *targetRelInfo, TupleTableSlot *slot, ResultRelInfo **partRelInfo); -static TupleTableSlot *internalGetUpdateNewTuple(ResultRelInfo *relinfo, - TupleTableSlot *planSlot, - TupleTableSlot *oldSlot, - MergeActionState *relaction); static TupleTableSlot *ExecMerge(ModifyTableContext *context, ResultRelInfo *resultRelInfo, @@ -179,10 +166,6 @@ static bool ExecMergeMatched(ModifyTable static void ExecMergeNotMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo, bool canSetTag); -static TupleTableSlot *mergeGetUpdateNewTuple(ResultRelInfo *relinfo, - TupleTableSlot *planSlot, - TupleTableSlot *oldSlot, - MergeActionState *relaction); /* @@ -736,30 +719,34 @@ ExecGetInsertNewTuple(ResultRelInfo *rel TupleTableSlot * ExecGetUpdateNewTuple(ResultRelInfo *relinfo, TupleTableSlot *planSlot, - TupleTableSlot *oldSlot) + TupleTableSlot *oldSlot, + MergeActionState *relaction) { + ProjectionInfo *newProj; + ExprContext *econtext; + /* Use a few extra Asserts to protect against outside callers */ Assert(relinfo->ri_projectNewInfoValid); Assert(planSlot != NULL && !TTS_EMPTY(planSlot)); Assert(oldSlot != NULL && !TTS_EMPTY(oldSlot)); - return internalGetUpdateNewTuple(relinfo, planSlot, oldSlot, NULL); -} - -/* - * Callback for ModifyTableState->GetUpdateNewTuple for use by regular UPDATE. - */ -static TupleTableSlot * -internalGetUpdateNewTuple(ResultRelInfo *relinfo, - TupleTableSlot *planSlot, - TupleTableSlot *oldSlot, - MergeActionState *relaction) -{ - ProjectionInfo *newProj = relinfo->ri_projectNew; - ExprContext *econtext; - - econtext = newProj->pi_exprContext; - econtext->ecxt_outertuple = planSlot; + /* + * For MERGE, targetlist Vars in UPDATE actions use INNER_VAR to refer to + * the source relation (see setrefs.c), so we must use ecxt_innertuple for + * tuples from the subplan. Otherwise, we use ecxt_outertuple. + */ + if (relaction) + { + newProj = relaction->mas_proj; + econtext = newProj->pi_exprContext; + econtext->ecxt_innertuple = planSlot; + } + else + { + newProj = relinfo->ri_projectNew; + econtext = newProj->pi_exprContext; + econtext->ecxt_outertuple = planSlot; + } econtext->ecxt_scantuple = oldSlot; return ExecProject(newProj); } @@ -1864,9 +1851,10 @@ ExecCrossPartitionUpdate(ModifyTableCont oldSlot)) elog(ERROR, "failed to fetch tuple being updated"); /* and project the new tuple to retry the UPDATE with */ - context->cpUpdateRetrySlot = - context->GetUpdateNewTuple(resultRelInfo, epqslot, oldSlot, - context->relaction); + context->cpUpdateRetrySlot = ExecGetUpdateNewTuple(resultRelInfo, + epqslot, + oldSlot, + context->relaction); return false; } } @@ -1931,7 +1919,7 @@ ExecUpdatePrologue(ModifyTableContext *c return ExecBRUpdateTriggers(context->estate, context->epqstate, resultRelInfo, tupleid, oldtuple, slot, - &context->tmfd); + &context->tmfd, context->relaction); } return true; @@ -2430,7 +2418,7 @@ redo_act: oldSlot)) elog(ERROR, "failed to fetch tuple being updated"); slot = ExecGetUpdateNewTuple(resultRelInfo, - epqslot, oldSlot); + epqslot, oldSlot, NULL); goto redo_act; case TM_Deleted: @@ -2902,7 +2890,6 @@ lmerge_matched: newslot = ExecProject(relaction->mas_proj); context->relaction = relaction; - context->GetUpdateNewTuple = mergeGetUpdateNewTuple; context->cpUpdateRetrySlot = NULL; if (!ExecUpdatePrologue(context, resultRelInfo, @@ -3397,25 +3384,6 @@ ExecInitMergeTupleSlots(ModifyTableState } /* - * Callback for ModifyTableContext->GetUpdateNewTuple for use by MERGE. It - * computes the updated tuple by projecting from the current merge action's - * projection. - */ -static TupleTableSlot * -mergeGetUpdateNewTuple(ResultRelInfo *relinfo, - TupleTableSlot *planSlot, - TupleTableSlot *oldSlot, - MergeActionState *relaction) -{ - ExprContext *econtext = relaction->mas_proj->pi_exprContext; - - econtext->ecxt_scantuple = oldSlot; - econtext->ecxt_innertuple = planSlot; - - return ExecProject(relaction->mas_proj); -} - -/* * Process BEFORE EACH STATEMENT triggers */ static void @@ -3870,9 +3838,8 @@ ExecModifyTable(PlanState *pstate) oldSlot)) elog(ERROR, "failed to fetch tuple being updated"); } - slot = internalGetUpdateNewTuple(resultRelInfo, context.planSlot, - oldSlot, NULL); - context.GetUpdateNewTuple = internalGetUpdateNewTuple; + slot = ExecGetUpdateNewTuple(resultRelInfo, context.planSlot, + oldSlot, NULL); context.relaction = NULL; /* Now apply the update. */ diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h new file mode 100644 index 67496d8..bad6818 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -232,7 +232,8 @@ extern bool ExecBRUpdateTriggers(EState ItemPointer tupleid, HeapTuple fdw_trigtuple, TupleTableSlot *newslot, - TM_FailureData *tmfd); + TM_FailureData *tmfd, + MergeActionState *relaction); extern void ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, ResultRelInfo *src_partinfo, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h new file mode 100644 index 946abc0..4ad3b6e --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -659,7 +659,8 @@ extern void CheckSubscriptionRelkind(cha */ extern TupleTableSlot *ExecGetUpdateNewTuple(ResultRelInfo *relinfo, TupleTableSlot *planSlot, - TupleTableSlot *oldSlot); + TupleTableSlot *oldSlot, + MergeActionState *relaction); extern ResultRelInfo *ExecLookupResultRelByOid(ModifyTableState *node, Oid resultoid, bool missing_ok, diff --git a/src/test/isolation/expected/merge-match-recheck.out b/src/test/isolation/expected/merge-match-recheck.out new file mode 100644 index 8183f52..63e614e --- a/src/test/isolation/expected/merge-match-recheck.out +++ b/src/test/isolation/expected/merge-match-recheck.out @@ -1,4 +1,4 @@ -Parsed test spec with 2 sessions +Parsed test spec with 3 sessions starting permutation: update1 merge_status c2 select1 c1 step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; @@ -22,6 +22,44 @@ key|balance|status|val (1 row) step c1: COMMIT; +s3: NOTICE: trigger "target_upd_trig" for relation "target" does not exist, skipping +s3: NOTICE: function target_upd_trig_fn() does not exist, skipping + +starting permutation: trigger update1 merge_status c2 select1 c1 +step trigger: + CREATE FUNCTION target_upd_trig_fn() RETURNS trigger LANGUAGE plpgsql AS + $$ + BEGIN + RAISE NOTICE 'Update target % -> %', OLD, NEW; + RETURN NEW; + END + $$; + CREATE TRIGGER target_upd_trig BEFORE UPDATE ON target + FOR EACH ROW EXECUTE FUNCTION target_upd_trig_fn(); + +s2: NOTICE: Update target (1,160,s1,setup) -> (1,170,s1,"setup updated by update1") +step update1: UPDATE target t SET balance = balance + 10, val = t.val || ' updated by update1' WHERE t.key = 1; +step merge_status: + MERGE INTO target t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND status = 's1' THEN + UPDATE SET status = 's2', val = t.val || ' when1' + WHEN MATCHED AND status = 's2' THEN + UPDATE SET status = 's3', val = t.val || ' when2' + WHEN MATCHED AND status = 's3' THEN + UPDATE SET status = 's4', val = t.val || ' when3'; + +step c2: COMMIT; +s1: NOTICE: Update target (1,170,s1,"setup updated by update1") -> (1,170,s2,"setup updated by update1 when1") +step merge_status: <... completed> +step select1: SELECT * FROM target; +key|balance|status|val +---+-------+------+------------------------------ + 1| 170|s2 |setup updated by update1 when1 +(1 row) + +step c1: COMMIT; starting permutation: update2 merge_status c2 select1 c1 step update2: UPDATE target t SET status = 's2', val = t.val || ' updated by update2' WHERE t.key = 1; @@ -45,6 +83,44 @@ key|balance|status|val (1 row) step c1: COMMIT; +s3: NOTICE: trigger "target_upd_trig" for relation "target" does not exist, skipping +s3: NOTICE: function target_upd_trig_fn() does not exist, skipping + +starting permutation: trigger update2 merge_status c2 select1 c1 +step trigger: + CREATE FUNCTION target_upd_trig_fn() RETURNS trigger LANGUAGE plpgsql AS + $$ + BEGIN + RAISE NOTICE 'Update target % -> %', OLD, NEW; + RETURN NEW; + END + $$; + CREATE TRIGGER target_upd_trig BEFORE UPDATE ON target + FOR EACH ROW EXECUTE FUNCTION target_upd_trig_fn(); + +s2: NOTICE: Update target (1,160,s1,setup) -> (1,160,s2,"setup updated by update2") +step update2: UPDATE target t SET status = 's2', val = t.val || ' updated by update2' WHERE t.key = 1; +step merge_status: + MERGE INTO target t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND status = 's1' THEN + UPDATE SET status = 's2', val = t.val || ' when1' + WHEN MATCHED AND status = 's2' THEN + UPDATE SET status = 's3', val = t.val || ' when2' + WHEN MATCHED AND status = 's3' THEN + UPDATE SET status = 's4', val = t.val || ' when3'; + +step c2: COMMIT; +s1: NOTICE: Update target (1,160,s2,"setup updated by update2") -> (1,160,s2,"setup updated by update2 when1") +step merge_status: <... completed> +step select1: SELECT * FROM target; +key|balance|status|val +---+-------+------+------------------------------ + 1| 160|s2 |setup updated by update2 when1 +(1 row) + +step c1: COMMIT; starting permutation: update3 merge_status c2 select1 c1 step update3: UPDATE target t SET status = 's3', val = t.val || ' updated by update3' WHERE t.key = 1; @@ -68,6 +144,8 @@ key|balance|status|val (1 row) step c1: COMMIT; +s3: NOTICE: trigger "target_upd_trig" for relation "target" does not exist, skipping +s3: NOTICE: function target_upd_trig_fn() does not exist, skipping starting permutation: update5 merge_status c2 select1 c1 step update5: UPDATE target t SET status = 's5', val = t.val || ' updated by update5' WHERE t.key = 1; @@ -91,6 +169,8 @@ key|balance|status|val (1 row) step c1: COMMIT; +s3: NOTICE: trigger "target_upd_trig" for relation "target" does not exist, skipping +s3: NOTICE: function target_upd_trig_fn() does not exist, skipping starting permutation: update_bal1 merge_bal c2 select1 c1 step update_bal1: UPDATE target t SET balance = 50, val = t.val || ' updated by update_bal1' WHERE t.key = 1; @@ -114,3 +194,41 @@ key|balance|status|val (1 row) step c1: COMMIT; +s3: NOTICE: trigger "target_upd_trig" for relation "target" does not exist, skipping +s3: NOTICE: function target_upd_trig_fn() does not exist, skipping + +starting permutation: trigger update_bal1 merge_bal c2 select1 c1 +step trigger: + CREATE FUNCTION target_upd_trig_fn() RETURNS trigger LANGUAGE plpgsql AS + $$ + BEGIN + RAISE NOTICE 'Update target % -> %', OLD, NEW; + RETURN NEW; + END + $$; + CREATE TRIGGER target_upd_trig BEFORE UPDATE ON target + FOR EACH ROW EXECUTE FUNCTION target_upd_trig_fn(); + +s2: NOTICE: Update target (1,160,s1,setup) -> (1,50,s1,"setup updated by update_bal1") +step update_bal1: UPDATE target t SET balance = 50, val = t.val || ' updated by update_bal1' WHERE t.key = 1; +step merge_bal: + MERGE INTO target t + USING (SELECT 1 as key) s + ON s.key = t.key + WHEN MATCHED AND balance < 100 THEN + UPDATE SET balance = balance * 2, val = t.val || ' when1' + WHEN MATCHED AND balance < 200 THEN + UPDATE SET balance = balance * 4, val = t.val || ' when2' + WHEN MATCHED AND balance < 300 THEN + UPDATE SET balance = balance * 8, val = t.val || ' when3'; + +step c2: COMMIT; +s1: NOTICE: Update target (1,50,s1,"setup updated by update_bal1") -> (1,200,s1,"setup updated by update_bal1 when2") +step merge_bal: <... completed> +step select1: SELECT * FROM target; +key|balance|status|val +---+-------+------+---------------------------------- + 1| 200|s1 |setup updated by update_bal1 when2 +(1 row) + +step c1: COMMIT; diff --git a/src/test/isolation/specs/merge-match-recheck.spec b/src/test/isolation/specs/merge-match-recheck.spec new file mode 100644 index d56400a..181a0a4 --- a/src/test/isolation/specs/merge-match-recheck.spec +++ b/src/test/isolation/specs/merge-match-recheck.spec @@ -61,11 +61,38 @@ step "update5" { UPDATE target t SET sta step "update_bal1" { UPDATE target t SET balance = 50, val = t.val || ' updated by update_bal1' WHERE t.key = 1; } step "c2" { COMMIT; } +session "s3" +step "trigger" +{ + CREATE FUNCTION target_upd_trig_fn() RETURNS trigger LANGUAGE plpgsql AS + $$ + BEGIN + RAISE NOTICE 'Update target % -> %', OLD, NEW; + RETURN NEW; + END + $$; + CREATE TRIGGER target_upd_trig BEFORE UPDATE ON target + FOR EACH ROW EXECUTE FUNCTION target_upd_trig_fn(); +} + +teardown +{ + DROP TRIGGER IF EXISTS target_upd_trig ON target; + DROP FUNCTION IF EXISTS target_upd_trig_fn; +} + # merge_status sees concurrently updated row and rechecks WHEN conditions, but recheck passes and final status = 's2' permutation "update1" "merge_status" "c2" "select1" "c1" +# adding a before row trigger doesn't change result +permutation "trigger" "update1" "merge_status" "c2" "select1" "c1" # merge_status sees concurrently updated row and rechecks WHEN conditions, recheck fails, so final status = 's3' not 's2' permutation "update2" "merge_status" "c2" "select1" "c1" +# XXX: merge_status fires before row trigger which locks the concurrently +# updated row after checking WHEN condition, so MERGE doesn't notice +# the concurrent update and doesn't recheck the WHEN condition, so +# final status = 's2' +permutation "trigger" "update2" "merge_status" "c2" "select1" "c1" # merge_status sees concurrently updated row and rechecks WHEN conditions, recheck fails, so final status = 's4' not 's2' permutation "update3" "merge_status" "c2" "select1" "c1" @@ -75,3 +102,9 @@ permutation "update5" "merge_status" "c2 # merge_bal sees concurrently updated row and rechecks WHEN conditions, recheck fails, so final balance = 100 not 640 permutation "update_bal1" "merge_bal" "c2" "select1" "c1" + +# XXX: merge_bal fires before row trigger which locks the concurrently +# updated row after checking WHEN condition, so MERGE doesn't notice +# the concurrent update and doesn't recheck the WHEN condition, so +# final balance = 200 +permutation "trigger" "update_bal1" "merge_bal" "c2" "select1" "c1"