Hi, I found another “wrong varnullingrels” error involving MERGE into a trigger-updatable view:
CREATE TABLE mt (id int, k int, v text); CREATE VIEW mv AS SELECT id, k, v FROM mt; CREATE FUNCTION mv_insert() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN RETURN NEW; END $$; CREATE TRIGGER mv_insert INSTEAD OF INSERT ON mv FOR EACH ROW EXECUTE FUNCTION mv_insert(); CREATE TABLE ms (id int, k int, v text); INSERT INTO ms VALUES (1, 1, 'source'); MERGE INTO mv t USING ms s RIGHT JOIN ms w ON s.k = w.k ON w.k = t.k AND t.k = 0 WHEN MATCHED AND s.k < t.id THEN DO NOTHING WHEN NOT MATCHED AND s.v IS NOT NULL THEN INSERT (v) VALUES (s.v); On the current master, this fails during planning with an error like: ERROR: wrong varnullingrels (b) (expected (b 6)) for Var 7/1 transform_MERGE_to_join() constructs a right join for the WHEN NOT MATCHED BY TARGET action, making the target relation a nullable side of the MERGE join. It marks target Vars in parse->targetList as nullable by that join, but does not do the same for target Vars in MERGE action expressions or the RETURNING list. In this example, the “t.id” Var from the MATCHED action qual is later added to the processed target list for evaluation by ModifyTable. It does not contain the MERGE join's nullingrel, while the corresponding Var produced by the child join does. The mismatch is eventually detected by setrefs.c. My initial fix mirrored the existing if (jointype == JOIN_LEFT || jointype == JOIN_FULL) block for right and full joins, using the target relation instead of the source relation. However, that duplicated most of the code for processing action quals, action target lists, and the RETURNING list. The attached patch instead refactors the existing handling. It first collects all relation RT indexes on nullable sides of the MERGE join: - source relids for a left join; - the target relid for a right join; - both for a full join. It then uses that set to mark Vars in the query target list, MERGE action expressions, and RETURNING list. For left and full joins, it also marks the copy of the merge join condition evaluated above the join for NOT MATCHED BY SOURCE processing. The original condition in the JoinExpr remains unchanged. Thoughts? -- Tender Wang
From 7f56eaaf19cbcb61d73b02e91bc70e8270d2bcd4 Mon Sep 17 00:00:00 2001 From: Tender Wang <[email protected]> Date: Fri, 13 Nov 2026 21:01:27 +0800 Subject: [PATCH] Fix nullingrels for expressions above MERGE joins When MERGE requires an outer join, Vars used above that join must be marked nullable if they belong to one of its nullable sides. transform_MERGE_to_join() handled this asymmetrically. For left and full joins, it marked source Vars in the merge join condition, action expressions, and RETURNING list. For right and full joins, however, it only marked target Vars in the query target list. Target Vars in action expressions and the RETURNING list remained unmarked. Such Vars are later added to the processed target list for evaluation by ModifyTable. This can make an upper plan node contain a target Var whose varnullingrels do not match the corresponding Var produced by its child, leading to a "wrong varnullingrels" error during setrefs processing. Refactor the code to first collect all relation RT indexes on nullable sides of the MERGE join. Use that set to mark Vars in the query target list, action expressions, and RETURNING list. For left and full joins, also apply it to the copy of the merge join condition evaluated above the join for NOT MATCHED BY SOURCE processing. Leave the original join condition in the JoinExpr unchanged. --- src/backend/optimizer/prep/prepjointree.c | 62 +++++++++++------------ src/test/regress/expected/merge.out | 22 ++++++++ src/test/regress/sql/merge.sql | 25 +++++++++ 3 files changed, 77 insertions(+), 32 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 0a9dd57b852..7db2e1e256c 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -214,6 +214,7 @@ transform_MERGE_to_join(Query *parse) FromExpr *target; Node *source; int sourcerti; + Relids nullable_relids = NULL; if (parse->commandType != CMD_MERGE) return; @@ -323,23 +324,25 @@ transform_MERGE_to_join(Query *parse) parse->jointree->quals = NULL; /* - * If necessary, mark parse->targetlist entries that refer to the target - * as nullable by the join. Normally the targetlist will be empty for a - * MERGE, but if the target is a trigger-updatable view, it will contain a - * whole-row Var referring to the expanded view query. + * Collect the RT indexes of relations on nullable sides of the join. */ - if (parse->targetList != NIL && - (jointype == JOIN_RIGHT || jointype == JOIN_FULL)) - parse->targetList = (List *) - add_nulling_relids((Node *) parse->targetList, - bms_make_singleton(parse->mergeTargetRelation), - bms_make_singleton(joinrti)); + if (jointype == JOIN_RIGHT || jointype == JOIN_FULL) + nullable_relids = + bms_add_member(nullable_relids, parse->mergeTargetRelation); + if (jointype == JOIN_LEFT || jointype == JOIN_FULL) + nullable_relids = + bms_join(nullable_relids, + get_relids_in_jointree(source, true, true)); /* - * If the source relation is on the outer side of the join, mark any - * source relation Vars in the join condition, actions, and RETURNING list - * as nullable by the join. These Vars will be added to the targetlist by - * preprocess_targetlist(), so it's important to mark them correctly here. + * Mark Vars used above the join as nullable by the join. This includes + * Vars in the query targetlist, action expressions, and RETURNING list. + * Vars from the latter expressions will be added to the targetlist by + * preprocess_targetlist(), so they must be marked correctly here. + * + * Normally the query targetlist is empty for MERGE, but for a + * trigger-updatable view it contains a whole-row Var referring to the + * expanded view query. * * It might seem that this is not necessary for Vars in the join * condition, since it is inside the join, but it is also needed above the @@ -348,38 +351,33 @@ transform_MERGE_to_join(Query *parse) * creates a modified copy of the join condition, for use above the join, * without modifying the original join condition, inside the join. */ - if (jointype == JOIN_LEFT || jointype == JOIN_FULL) + if (nullable_relids != NULL) { - Relids sourcerelids; - - /* - * If the source relation is a join, we must mark any Vars referring - * to relations under the join as well as the join relation itself. - */ - sourcerelids = get_relids_in_jointree(source, true, true); + Relids nullingrel = bms_make_singleton(joinrti); - parse->mergeJoinCondition = - add_nulling_relids(parse->mergeJoinCondition, - sourcerelids, - bms_make_singleton(joinrti)); + parse->targetList = (List *) + add_nulling_relids((Node *) parse->targetList, + nullable_relids, nullingrel); foreach_node(MergeAction, action, parse->mergeActionList) { action->qual = add_nulling_relids(action->qual, - sourcerelids, - bms_make_singleton(joinrti)); + nullable_relids, nullingrel); action->targetList = (List *) add_nulling_relids((Node *) action->targetList, - sourcerelids, - bms_make_singleton(joinrti)); + nullable_relids, nullingrel); } parse->returningList = (List *) add_nulling_relids((Node *) parse->returningList, - sourcerelids, - bms_make_singleton(joinrti)); + nullable_relids, nullingrel); + + if (jointype == JOIN_LEFT || jointype == JOIN_FULL) + parse->mergeJoinCondition = + add_nulling_relids(parse->mergeJoinCondition, + nullable_relids, nullingrel); } /* diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out index 905c3fe8cb1..ea743e89418 100644 --- a/src/test/regress/expected/merge.out +++ b/src/test/regress/expected/merge.out @@ -23,6 +23,28 @@ SELECT t.ctid is not null as matched, t.*, s.* FROM source s FULL OUTER JOIN tar t | 3 | 30 | | (3 rows) +-- target Vars in action expressions above a right MERGE join +CREATE VIEW merge_target_view AS + SELECT tid, balance FROM target; +CREATE FUNCTION merge_target_view_insert() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + RETURN NEW; +END +$$; +CREATE TRIGGER merge_target_view_insert + INSTEAD OF INSERT ON merge_target_view + FOR EACH ROW EXECUTE FUNCTION merge_target_view_insert(); +INSERT INTO source VALUES (1, 1); +MERGE INTO merge_target_view t +USING source s RIGHT JOIN source w ON s.sid = w.sid +ON w.sid = t.tid AND t.tid = 0 +WHEN MATCHED AND s.sid < t.tid THEN DO NOTHING +WHEN NOT MATCHED AND s.delta IS NOT NULL THEN + INSERT (balance) VALUES (s.delta); +TRUNCATE source; +DROP VIEW merge_target_view; +DROP FUNCTION merge_target_view_insert(); ALTER TABLE target OWNER TO regress_merge_privs; ALTER TABLE source OWNER TO regress_merge_privs; CREATE TABLE target2 (tid integer, balance integer) diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql index 37aef4de68a..3b1a1db8d66 100644 --- a/src/test/regress/sql/merge.sql +++ b/src/test/regress/sql/merge.sql @@ -17,6 +17,31 @@ INSERT INTO target VALUES (2, 20); INSERT INTO target VALUES (3, 30); SELECT t.ctid is not null as matched, t.*, s.* FROM source s FULL OUTER JOIN target t ON s.sid = t.tid ORDER BY t.tid, s.sid; +-- target Vars in action expressions above a right MERGE join +CREATE VIEW merge_target_view AS + SELECT tid, balance FROM target; +CREATE FUNCTION merge_target_view_insert() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + RETURN NEW; +END +$$; +CREATE TRIGGER merge_target_view_insert + INSTEAD OF INSERT ON merge_target_view + FOR EACH ROW EXECUTE FUNCTION merge_target_view_insert(); + +INSERT INTO source VALUES (1, 1); +MERGE INTO merge_target_view t +USING source s RIGHT JOIN source w ON s.sid = w.sid +ON w.sid = t.tid AND t.tid = 0 +WHEN MATCHED AND s.sid < t.tid THEN DO NOTHING +WHEN NOT MATCHED AND s.delta IS NOT NULL THEN + INSERT (balance) VALUES (s.delta); + +TRUNCATE source; +DROP VIEW merge_target_view; +DROP FUNCTION merge_target_view_insert(); + ALTER TABLE target OWNER TO regress_merge_privs; ALTER TABLE source OWNER TO regress_merge_privs; -- 2.43.0
