## Draft pgsql-bugs report (NOT SENT; the user decides whether to send it) Form fields as on postgresql.org/account/submitbug/. Revised after the #942 bisect: names the commit that caused the regression, quotes the documentation on partitioned-table transition relations, reports the older double-capture half, and inlines everything a maintainer needs, including a candidate patch. **Your name / email:** Michael Ries / mries@salesforce.com **PostgreSQL version number you are running:** 15.6, 15.19, 16.15, 17.11 and 18.6 behave as described below. 15.5 behaves differently (described below). The bisect (REL_15 `c0bfdaf2b`, REL_16 `06a546382`) was run on builds from git. The `master` source (at `f884f359f5`) has the same code; not built. **How you installed PostgreSQL:** 15.5, 15.6, 15.19, 16.15 and 18.6 built from the release tarballs with the default configuration (gcc 16.2.1; 15.5 and 15.6 with `CFLAGS=-std=gnu17`). The bisect builds are from the git repository with the default configuration and `CFLAGS=-std=gnu17`. 17.11 from the Fedora package. **Changes made to the settings in the postgresql.conf file:** `wal_level = logical`, `fsync = off`, `max_connections = 50`. None of these matters for the report. **Operating system and version:** Linux 7.2.8 (Fedora 44), x86_64. **What program you're using to connect to PostgreSQL:** psql. **Is there anything relevant or unusual in the PostgreSQL server logs?:** No. There are no errors; the statements succeed. **Short description:** `MERGE` whose `UPDATE` action moves a row to another partition leaves it out of the `UPDATE` transition tables (regression in 15.6/16.2, from commit c0bfdaf2b / 06a546382) **Details:** When a `MERGE`'s `UPDATE` action changes the partition key, so the row moves to another partition, the statement-level `AFTER UPDATE` trigger's transition tables don't contain that row on 15.6 and later (including 16.15, 17.11 and 18.6). On 15.5 they do. A plain `UPDATE` that moves a row puts its old and new images in the `UPDATE` transition tables on every version. The change in behaviour is introduced by commit `c0bfdaf2b` on REL_15 (`06a546382` on REL_16, `a4f7d33a9` on master), "Fix AFTER ROW trigger execution in MERGE cross-partition update." (Dean Rasheed, 2023-11-09). I built the parent and the commit from git on each branch and ran the script below: the parent gives `UPDATE` old 1 / new 1 for case B, and the commit gives `UPDATE` old 0 / new 0. The other commit in the 15.5..15.6 range that touches this code, the BUG #18238 fix (`7e8c6d7af` on REL_15, `7f07384dc` on REL_16), makes no difference to any of the results below. There is also an older, related problem. When the `MERGE` lists an `INSERT` action, the moved row's new image appears in the `INSERT` transition table; when it lists a `DELETE` action, its old image appears in the `DELETE` transition table. That happens even when the listed action matches no row (for example `WHEN MATCHED AND false THEN DELETE`). Before `c0bfdaf2b` that duplicated the image, since it was also in the `UPDATE` tables. Since `c0bfdaf2b` it is the only place the image appears. | `MERGE` actions (one row moves) | before `c0bfdaf2b` / `06a546382` (e.g. 15.5) | at and after it (15.6 and later) | |---|---|---| | `UPDATE` only | correct: `UPDATE` old 1 / new 1 | `UPDATE` old 0 / new 0 | | `UPDATE` + `INSERT` | new image in `UPDATE` new **and** in `INSERT` new | new image only in `INSERT` new; old image nowhere | | `UPDATE` + `DELETE` | old image in `UPDATE` old **and** in `DELETE` old | old image only in `DELETE` old; new image nowhere | | `UPDATE` + `DELETE` + `INSERT` | both images twice | images only in `DELETE` old and `INSERT` new | Reproduction (self-contained; the trigger functions only count rows): ```sql create table mr (id int, k int, v int) partition by range (k); create table mr_a partition of mr for values from (0) to (10); create table mr_b partition of mr for values from (10) to (20); create table log (seq serial, trg text, n_old int, n_new int); create function on_upd() returns trigger language plpgsql as $$ begin insert into log (trg, n_old, n_new) values (tg_op, (select count(*) from ot), (select count(*) from nt)); return null; end $$; create function on_ins() returns trigger language plpgsql as $$ begin insert into log (trg, n_new) values (tg_op, (select count(*) from nt)); return null; end $$; create function on_del() returns trigger language plpgsql as $$ begin insert into log (trg, n_old) values (tg_op, (select count(*) from ot)); return null; end $$; create trigger t_upd after update on mr referencing old table as ot new table as nt for each statement execute function on_upd(); create trigger t_ins after insert on mr referencing new table as nt for each statement execute function on_ins(); create trigger t_del after delete on mr referencing old table as ot for each statement execute function on_del(); insert into mr values (1, 1, 100), (2, 2, 200); -- both in mr_a delete from log; -- A: plain UPDATE that moves id 1 to mr_b update mr set k = 11 where id = 1; select * from log order by seq; -- UPDATE | old 1 | new 1 (all versions) truncate log; truncate mr; insert into mr values (1, 1, 100), (2, 2, 200); delete from log; -- B: MERGE UPDATE that moves id 1 to mr_b merge into mr using (values (1)) s(id) on mr.id = s.id when matched then update set k = 11; select * from log order by seq; -- 15.6+: UPDATE | old 0 | new 0 (15.5: 1 | 1) truncate log; truncate mr; insert into mr values (1, 1, 100), (2, 2, 200); delete from log; -- D: MERGE with UPDATE + INSERT actions (moves id 1, inserts id 9) merge into mr using (values (1), (9)) s(id) on mr.id = s.id when matched then update set k = 11 when not matched then insert values (s.id, 4, 0); select * from log order by seq; -- 15.6+: UPDATE 0/0, INSERT new 2 (id 9 plus the moved row) -- 15.5: UPDATE 1/1, INSERT new 2 truncate log; truncate mr; insert into mr values (1, 1, 100), (2, 2, 200); delete from log; -- E: MERGE with UPDATE + DELETE actions (moves id 1, deletes id 2) merge into mr using (values (1), (2)) s(id) on mr.id = s.id when matched and s.id = 2 then delete when matched then update set k = 11; select * from log order by seq; -- 15.6+: DELETE old 2 (id 2 plus the moved row), UPDATE 0/0 -- 15.5: DELETE old 2, UPDATE 1/1 ``` Expected result: the moved row's old and new images appear in the `UPDATE` transition tables, as for a plain `UPDATE`, and in no other transition table. The `CREATE TRIGGER` documentation (section "Notes on partitioned tables") says: "If a statement-level trigger has been defined with transition relations named by a REFERENCING clause, then before and after images of rows are visible from all affected partitions or child tables." Together with its definitions, `OLD TABLE` "creates a transition relation containing the before-images of all rows updated or deleted by the statement" and `NEW TABLE` "creates a transition relation containing the after-images of all rows updated or inserted by the statement", and "an `INSERT` trigger will see only the inserted rows, while an `UPDATE` trigger will see only the updated rows", a statement-level trigger on the named table should see the images of every row the statement touched, in every partition. A row that a `MERGE` `UPDATE` action moved was updated by the statement. The page does not say anything specific about `MERGE` and row movement. Actual result: as in the table above. It is the same for RANGE, LIST and HASH partitioning, with a DEFAULT partition, with sub-partitioning, and when the `MERGE` names an intermediate partitioned table. On 17 and 18, a `WHEN NOT MATCHED BY SOURCE … UPDATE` action that moves a row behaves the same. A `MERGE` update that keeps the row in its partition, and `MERGE` on an ordinary table or an inheritance parent, are correct. Analysis (bisected to the commit; the mechanism is from reading the source, not from a debugger): - Bisect: `c0bfdaf2b^` (`456d697ba`) gives `UPDATE` 1/1 for case B and `c0bfdaf2b` gives 0/0. The same flip occurs at `06a546382^` (`ef7c36555`) / `06a546382` on REL_16. The BUG #18238 fix `7e8c6d7af` (parent `3e8379aff`, already past `c0bfdaf2b`) shows 0/0 on both sides. A 270-case matrix of table kinds, action sets and scenarios gives identical results on both branches. At the parent all 63 wrong cases have a spurious (duplicated) image and none a missing one; at the commit all 63 have a missing image and none a spurious one. - The commit adds, in `ExecMergeMatched`, an early exit right after `ExecUpdateAct` reports `updateCxt.crossPartUpdate`, so that `ExecUpdateEpilogue` no longer runs for the moved row (it fired AFTER UPDATE ROW triggers, which should not fire for a cross-partition move, as for a plain `UPDATE`). Before the commit, `ExecUpdateEpilogue` ran and, as far as I can tell, was what captured the pair into the `UPDATE` transition tables. - The older half: in `nodeModifyTable.c`, `ExecInsert` and `ExecDeleteEpilogue` route a moved row's images into the `UPDATE` transition tables only when `mtstate->operation == CMD_UPDATE`. For `MERGE` the operation is `CMD_MERGE`, so the delete and insert halves go through the ordinary `ExecARDeleteTriggers` / `ExecARInsertTriggers` capture into the `DELETE`/`INSERT` tuplestores, which exist whenever a matching trigger with `REFERENCING` exists. - A candidate fix, tried on 15.19 (scratch copy, not submitted): treat a `MERGE` `UPDATE` action like `CMD_UPDATE` in those two guards, and always suppress the `INSERT`/`DELETE` capture for the moved row. Result: all 198 non-error cases of the matrix are exact (stock 15.19: 63 wrong), my script's cases A to J are exact, the cases where only `INSERT`/`DELETE` transition triggers exist are exact, row-level AFTER triggers fire as before, and `make check` passes (217 tests). It does not bring back AFTER UPDATE ROW triggers firing for the moved row. I did not test REL_16 and later, the isolation suite, the concurrent-update retry path, foreign tables, or leaves with a different column order.
Candidate patch (REL_15_19, nodeModifyTable.c) ```diff --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -739,6 +739,22 @@ return ExecProject(newProj); } +/* + * MergeUpdateActionInProgress -- is the MERGE action being executed an UPDATE? + * + * A MERGE UPDATE action that moves a row to another partition runs + * ExecDeleteEpilogue() and ExecInsert() for the two halves of the move, just + * as a plain UPDATE does, and the row's images belong in the UPDATE + * transition tables, not the DELETE/INSERT ones. + */ +static inline bool +MergeUpdateActionInProgress(ModifyTableContext *context) +{ + return context->mtstate->operation == CMD_MERGE && + context->relaction != NULL && + context->relaction->mas_action->commandType == CMD_UPDATE; +} + /* ---------------------------------------------------------------- * ExecInsert * @@ -1161,21 +1177,24 @@ * because they happen on different tables. */ ar_insert_trig_tcs = mtstate->mt_transition_capture; - if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture - && mtstate->mt_transition_capture->tcs_update_new_table) + if (mtstate->operation == CMD_UPDATE || + MergeUpdateActionInProgress(context)) { - ExecARUpdateTriggers(estate, resultRelInfo, - NULL, NULL, - NULL, - NULL, - slot, - NULL, - mtstate->mt_transition_capture, - false); - - /* - * We've already captured the NEW TABLE row, so make sure any AR - * INSERT trigger fired below doesn't capture it again. + if (mtstate->mt_transition_capture && + mtstate->mt_transition_capture->tcs_update_new_table) + ExecARUpdateTriggers(estate, resultRelInfo, + NULL, NULL, + NULL, + NULL, + slot, + NULL, + mtstate->mt_transition_capture, + false); + + /* + * The NEW TABLE row is captured above, or no UPDATE trigger wants + * it, so make sure any AR INSERT trigger fired below doesn't capture + * it as an inserted row. */ ar_insert_trig_tcs = NULL; } @@ -1382,18 +1401,21 @@ * because they happen on different tables. */ ar_delete_trig_tcs = mtstate->mt_transition_capture; - if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture && - mtstate->mt_transition_capture->tcs_update_old_table) + if (mtstate->operation == CMD_UPDATE || + (mtstate->operation == CMD_MERGE && changingPart)) { - ExecARUpdateTriggers(estate, resultRelInfo, - NULL, NULL, - tupleid, oldtuple, - NULL, NULL, mtstate->mt_transition_capture, - false); + if (mtstate->mt_transition_capture && + mtstate->mt_transition_capture->tcs_update_old_table) + ExecARUpdateTriggers(estate, resultRelInfo, + NULL, NULL, + tupleid, oldtuple, + NULL, NULL, mtstate->mt_transition_capture, + false); /* - * We've already captured the OLD TABLE row, so make sure any AR - * DELETE trigger fired below doesn't capture it again. + * The OLD TABLE row is captured above, or no UPDATE trigger wants + * it, so make sure any AR DELETE trigger fired below doesn't capture + * it as a deleted row. */ ar_delete_trig_tcs = NULL; } ```
Related: https://www.postgresql.org/message-id/CAEZATCWjBgagyNZs02vgDF0DvASYj-iHTFtXG2-nP3orZhmtcw%40mail.gmail.com ; BUG #18238 (`BEFORE ROW DELETE` trigger in cross-partition `MERGE` updates; fixed by `7e8c6d7af` / `7f07384dc`, not involved here); BUG #19380 (`MERGE` transition-table capture in CTE queries, fixed in 15.16 / 16.12 / 17.8 / 18.2). The regression test added by `c0bfdaf2b` is row-level only, and the `triggers.sql` tests cover `MERGE` with transition tables only on non-partitioned tables. I found no existing report of this behaviour.