From e3ac85014eb280ee2e82b36dc3be1b62c838b3e4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 15 Mar 2023 11:59:18 -0400 Subject: [PATCH] Support PlaceHolderVars in MERGE actions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preprocess_targetlist thought PHVs couldn't appear here. It was mistaken, as per report from Önder Kalacı. Surveying other pull_var_clause calls, I noted no similar errors, but I did notice that qual_is_pushdown_safe's assertion about !contain_window_function was pointless, because the following pull_var_clause call would complain about them anyway. In HEAD only, remove the redundant Assert and improve the commentary. Discussion: https://postgr.es/m/CACawEhUuum-gC_2S3sXLTcsk7bUSPSHOD+g1ZpfKaDK-KKPPWA@mail.gmail.com --- src/backend/optimizer/path/allpaths.c | 12 +++++------- src/backend/optimizer/prep/preptlist.c | 12 +++++------- src/test/regress/expected/merge.out | 21 +++++++++++++++++++++ src/test/regress/sql/merge.sql | 10 ++++++++++ 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 479a694bcac..132252b3e44 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -3839,18 +3839,16 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo, contain_leaked_vars(qual)) return false; - /* - * It would be unsafe to push down window function calls, but at least for - * the moment we could never see any in a qual anyhow. (The same applies - * to aggregates, which we check for in pull_var_clause below.) - */ - Assert(!contain_window_function(qual)); - /* * Examine all Vars used in clause. Since it's a restriction clause, all * such Vars must refer to subselect output columns ... unless this is * part of a LATERAL subquery, in which case there could be lateral * references. + * + * By omitting the relevant flags, this also gives us a cheap sanity check + * that no aggregates or window functions appear in the qual. Those would + * be unsafe to push down, but at least for the moment we could never see + * any in a qual anyhow. */ vars = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS); foreach(vl, vars) diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index c6d747b275c..2615d7f0b33 100644 --- a/src/backend/optimizer/prep/preptlist.c +++ b/src/backend/optimizer/prep/preptlist.c @@ -155,17 +155,15 @@ preprocess_targetlist(PlannerInfo *root) extract_update_targetlist_colnos(action->targetList); /* - * Add resjunk entries for any Vars used in each action's - * targetlist and WHEN condition that belong to relations other - * than target. Note that aggregates, window functions and - * placeholder vars are not possible anywhere in MERGE's WHEN - * clauses. (PHVs may be added later, but they don't concern us - * here.) + * Add resjunk entries for any Vars and PlaceHolderVars used in + * each action's targetlist and WHEN condition that belong to + * relations other than the target. We don't expect to see any + * aggregates or window functions here. */ vars = pull_var_clause((Node *) list_concat_copy((List *) action->qual, action->targetList), - 0); + PVC_INCLUDE_PLACEHOLDERS); foreach(l2, vars) { Var *var = (Var *) lfirst(l2); diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out index e32afc3b0c0..1ddc68b7895 100644 --- a/src/test/regress/expected/merge.out +++ b/src/test/regress/expected/merge.out @@ -1905,6 +1905,27 @@ SELECT * FROM cj_target; 2 | 320 | initial source2 300 (4 rows) +-- try it with an outer join and PlaceHolderVar +MERGE INTO cj_target t +USING (SELECT *, 'join input'::text AS phv FROM cj_source1) fj + FULL JOIN cj_source2 fj2 ON fj.scat = fj2.sid2 * 10 +ON t.tid = fj.scat +WHEN NOT MATCHED THEN + INSERT (tid, balance, val) VALUES (fj.scat, fj.delta, fj.phv); +SELECT * FROM cj_target; + tid | balance | val +-----+---------+---------------------------------- + 3 | 400 | initial source2 updated by merge + 1 | 220 | initial source2 200 + 1 | 110 | initial source2 200 + 2 | 320 | initial source2 300 + 10 | 100 | join input + 10 | 400 | join input + 20 | 200 | join input + 20 | 300 | join input + | | +(9 rows) + ALTER TABLE cj_source1 RENAME COLUMN sid1 TO sid; ALTER TABLE cj_source2 RENAME COLUMN sid2 TO sid; TRUNCATE cj_target; diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql index cae6902f2c3..29a35486d01 100644 --- a/src/test/regress/sql/merge.sql +++ b/src/test/regress/sql/merge.sql @@ -1225,6 +1225,16 @@ WHEN MATCHED THEN SELECT * FROM cj_target; +-- try it with an outer join and PlaceHolderVar +MERGE INTO cj_target t +USING (SELECT *, 'join input'::text AS phv FROM cj_source1) fj + FULL JOIN cj_source2 fj2 ON fj.scat = fj2.sid2 * 10 +ON t.tid = fj.scat +WHEN NOT MATCHED THEN + INSERT (tid, balance, val) VALUES (fj.scat, fj.delta, fj.phv); + +SELECT * FROM cj_target; + ALTER TABLE cj_source1 RENAME COLUMN sid1 TO sid; ALTER TABLE cj_source2 RENAME COLUMN sid2 TO sid;