pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Fix concurrent update trigger issues with MERGE in a CTE. 3+ messages / 1 participants [nested] [flat]
* pgsql: Fix concurrent update trigger issues with MERGE in a CTE. @ 2025-07-18 09:03 Dean Rasheed <dean.a.rasheed@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Dean Rasheed @ 2025-07-18 09:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix concurrent update trigger issues with MERGE in a CTE. If a MERGE inside a CTE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, and a concurrent UPDATE or DELETE happens, the merge code would fail (crashing in the case of an UPDATE action, and potentially executing the wrong action for a DELETE action). This is the same issue that 9321c79c86 attempted to fix, except now for a MERGE inside a CTE. As noted in 9321c79c86, what needs to happen is for the trigger code to exit early, returning the TM_Result and TM_FailureData information to the merge code, if a concurrent modification is detected, rather than attempting to do an EPQ recheck. The merge code will then do its own rechecking, and rescan the action list, potentially executing a different action in light of the concurrent update. In particular, the trigger code must never call ExecGetUpdateNewTuple() for MERGE, since that is bound to fail because MERGE has its own per-action projection information. Commit 9321c79c86 did this using estate->es_plannedstmt->commandType in the trigger code to detect that a MERGE was being executed, which is fine for a plain MERGE command, but does not work for a MERGE inside a CTE. Fix by passing that information to the trigger code as an additional parameter passed to ExecBRUpdateTriggers() and ExecBRDeleteTriggers(). Back-patch as far as v17 only, since MERGE cannot appear inside a CTE prior to that. Additionally, take care to preserve the trigger ABI in v17 (though not in v18, which is still in beta). Bug: #18986 Reported-by: Yaroslav Syrytsia <me@ys.lc> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18986-e7a8aac3d339fa47@postgresql.org Backpatch-through: 17 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/5022ff250eeba2367fb4e74fed8ee65bcddb6c99 Modified Files -------------- src/backend/commands/trigger.c | 74 ++++++++++++++-------- src/backend/executor/execReplication.c | 4 +- src/backend/executor/nodeModifyTable.c | 6 +- src/include/commands/trigger.h | 6 +- .../isolation/expected/merge-match-recheck.out | 27 +++++--- src/test/isolation/specs/merge-match-recheck.spec | 22 ++++--- 6 files changed, 89 insertions(+), 50 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix concurrent update trigger issues with MERGE in a CTE. @ 2025-07-18 09:03 Dean Rasheed <dean.a.rasheed@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Dean Rasheed @ 2025-07-18 09:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix concurrent update trigger issues with MERGE in a CTE. If a MERGE inside a CTE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, and a concurrent UPDATE or DELETE happens, the merge code would fail (crashing in the case of an UPDATE action, and potentially executing the wrong action for a DELETE action). This is the same issue that 9321c79c86 attempted to fix, except now for a MERGE inside a CTE. As noted in 9321c79c86, what needs to happen is for the trigger code to exit early, returning the TM_Result and TM_FailureData information to the merge code, if a concurrent modification is detected, rather than attempting to do an EPQ recheck. The merge code will then do its own rechecking, and rescan the action list, potentially executing a different action in light of the concurrent update. In particular, the trigger code must never call ExecGetUpdateNewTuple() for MERGE, since that is bound to fail because MERGE has its own per-action projection information. Commit 9321c79c86 did this using estate->es_plannedstmt->commandType in the trigger code to detect that a MERGE was being executed, which is fine for a plain MERGE command, but does not work for a MERGE inside a CTE. Fix by passing that information to the trigger code as an additional parameter passed to ExecBRUpdateTriggers() and ExecBRDeleteTriggers(). Back-patch as far as v17 only, since MERGE cannot appear inside a CTE prior to that. Additionally, take care to preserve the trigger ABI in v17 (though not in v18, which is still in beta). Bug: #18986 Reported-by: Yaroslav Syrytsia <me@ys.lc> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18986-e7a8aac3d339fa47@postgresql.org Backpatch-through: 17 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/91ad1bdef8e46983d82d2723c6e1c05e004f74c5 Modified Files -------------- src/backend/commands/trigger.c | 134 +++++++++++++++------ src/backend/executor/nodeModifyTable.c | 14 ++- src/include/commands/trigger.h | 18 +++ .../isolation/expected/merge-match-recheck.out | 27 +++-- src/test/isolation/specs/merge-match-recheck.spec | 22 ++-- 5 files changed, 153 insertions(+), 62 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
* pgsql: Fix concurrent update trigger issues with MERGE in a CTE. @ 2025-07-18 09:03 Dean Rasheed <dean.a.rasheed@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Dean Rasheed @ 2025-07-18 09:03 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Fix concurrent update trigger issues with MERGE in a CTE. If a MERGE inside a CTE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, and a concurrent UPDATE or DELETE happens, the merge code would fail (crashing in the case of an UPDATE action, and potentially executing the wrong action for a DELETE action). This is the same issue that 9321c79c86 attempted to fix, except now for a MERGE inside a CTE. As noted in 9321c79c86, what needs to happen is for the trigger code to exit early, returning the TM_Result and TM_FailureData information to the merge code, if a concurrent modification is detected, rather than attempting to do an EPQ recheck. The merge code will then do its own rechecking, and rescan the action list, potentially executing a different action in light of the concurrent update. In particular, the trigger code must never call ExecGetUpdateNewTuple() for MERGE, since that is bound to fail because MERGE has its own per-action projection information. Commit 9321c79c86 did this using estate->es_plannedstmt->commandType in the trigger code to detect that a MERGE was being executed, which is fine for a plain MERGE command, but does not work for a MERGE inside a CTE. Fix by passing that information to the trigger code as an additional parameter passed to ExecBRUpdateTriggers() and ExecBRDeleteTriggers(). Back-patch as far as v17 only, since MERGE cannot appear inside a CTE prior to that. Additionally, take care to preserve the trigger ABI in v17 (though not in v18, which is still in beta). Bug: #18986 Reported-by: Yaroslav Syrytsia <me@ys.lc> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18986-e7a8aac3d339fa47@postgresql.org Backpatch-through: 17 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/27c7c11366f72b1933e298481954a24c742036de Modified Files -------------- src/backend/commands/trigger.c | 74 ++++++++++++++-------- src/backend/executor/execReplication.c | 4 +- src/backend/executor/nodeModifyTable.c | 6 +- src/include/commands/trigger.h | 6 +- .../isolation/expected/merge-match-recheck.out | 27 +++++--- src/test/isolation/specs/merge-match-recheck.spec | 22 ++++--- 6 files changed, 89 insertions(+), 50 deletions(-) ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-07-18 09:03 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-07-18 09:03 pgsql: Fix concurrent update trigger issues with MERGE in a CTE. Dean Rasheed <dean.a.rasheed@gmail.com> 2025-07-18 09:03 pgsql: Fix concurrent update trigger issues with MERGE in a CTE. Dean Rasheed <dean.a.rasheed@gmail.com> 2025-07-18 09:03 pgsql: Fix concurrent update trigger issues with MERGE in a CTE. Dean Rasheed <dean.a.rasheed@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox