Support PlaceHolderVars in MERGE actions.

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

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 479a694bcac1192a58608c0d3cbe2eef08a29a99..132252b3e44ede9cc974009820cd83052da9c537 100644 (file)
--- 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 c6d747b275c2fe1e465ae77d5dda7ea87f23660e..2615d7f0b33d8a1d7520e3684257f6a2c07eaa71 100644 (file)
--- 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 e32afc3b0c0fb42fd630cc85aaa084bfce74f82b..1ddc68b78955325293d82baa7810204164745869 100644 (file)
--- 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 cae6902f2c327773a2e876d368625e8da779bb51..29a35486d01d26af96bc59baaf19d8a869193428 100644 (file)
--- 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;