Change mechanism to set up source targetlist in MERGE
authorAlvaro Herrera <[email protected]>
Tue, 12 Apr 2022 07:29:39 +0000 (09:29 +0200)
committerAlvaro Herrera <[email protected]>
Tue, 12 Apr 2022 07:29:39 +0000 (09:29 +0200)
We were setting MERGE source subplan's targetlist by expanding the
individual attributes of the source relation completely, early in the
parse analysis phase.  This failed to work when the condition of an
action included a whole-row reference, causing setrefs.c to error out
with
  ERROR:  variable not found in subplan target lists
because at that point there is nothing to resolve the whole-row
reference with.  We can fix this by having preprocess_targetlist expand
the source targetlist for Vars required from the source rel by all
actions.  Moreover, by using this expansion mechanism we can do away
with the targetlist expansion in transformMergeStmt, which is good
because then we no longer pull in columns that aren't needed for
anything.

Add a test case for the problem.

While at it, remove some redundant code in preprocess_targetlist():
MERGE was doing separately what is already being done for UPDATE/DELETE,
so we can just rely on the latter and remove the former.  (The handling
of inherited rels was different for MERGE, but that was a no-longer-
necessary hack.)

Fix outdated, related comments for fix_join_expr also.

Author: Richard Guo <[email protected]>
Author: Álvaro Herrera <[email protected]>
Reported-by: Joe Wildish <[email protected]>
Discussion: https://postgr.es/m/fab3b90a-914d-46a9-beb0-df011ee39ee5@www.fastmail.com

src/backend/optimizer/plan/setrefs.c
src/backend/optimizer/prep/preptlist.c
src/backend/parser/parse_merge.c
src/test/regress/expected/merge.out
src/test/regress/expected/with.out
src/test/regress/sql/merge.sql

index 6ea35056465c946c0b8f950bc8bd769d1ee42a35..d95fd8980778200e5ecfb4bc5f0b506da164671a 100644 (file)
@@ -2748,7 +2748,7 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
  *        relation target lists.  Also perform opcode lookup and add
  *        regclass OIDs to root->glob->relationOids.
  *
- * This is used in three different scenarios:
+ * This is used in four different scenarios:
  * 1) a normal join clause, where all the Vars in the clause *must* be
  *       replaced by OUTER_VAR or INNER_VAR references.  In this case
  *       acceptable_rel should be zero so that any failure to match a Var will be
@@ -2763,6 +2763,11 @@ search_indexed_tlist_for_sortgroupref(Expr *node,
  *       to-be-updated relation) alone. Correspondingly inner_itlist is to be
  *       EXCLUDED elements, outer_itlist = NULL and acceptable_rel the target
  *       relation.
+ * 4) MERGE.  In this case, references to the source relation are to be
+ *    replaced with INNER_VAR references, leaving Vars of the target
+ *    relation (the to-be-modified relation) alone.  So inner_itlist is to be
+ *    the source relation elements, outer_itlist = NULL and acceptable_rel
+ *    the target relation.
  *
  * 'clauses' is the targetlist or list of join clauses
  * 'outer_itlist' is the indexed target list of the outer join relation,
index 99ab3d75594666b305cbcf9041610dec9b4b8f48..137b28323d6429f398a2b2c535da20609078a7e7 100644 (file)
@@ -107,14 +107,15 @@ preprocess_targetlist(PlannerInfo *root)
                root->update_colnos = extract_update_targetlist_colnos(tlist);
 
        /*
-        * For non-inherited UPDATE/DELETE, register any junk column(s) needed to
-        * allow the executor to identify the rows to be updated or deleted.  In
-        * the inheritance case, we do nothing now, leaving this to be dealt with
-        * when expand_inherited_rtentry() makes the leaf target relations.  (But
-        * there might not be any leaf target relations, in which case we must do
-        * this in distribute_row_identity_vars().)
+        * For non-inherited UPDATE/DELETE/MERGE, register any junk column(s)
+        * needed to allow the executor to identify the rows to be updated or
+        * deleted.  In the inheritance case, we do nothing now, leaving this to
+        * be dealt with when expand_inherited_rtentry() makes the leaf target
+        * relations.  (But there might not be any leaf target relations, in which
+        * case we must do this in distribute_row_identity_vars().)
         */
-       if ((command_type == CMD_UPDATE || command_type == CMD_DELETE) &&
+       if ((command_type == CMD_UPDATE || command_type == CMD_DELETE ||
+                command_type == CMD_MERGE) &&
                !target_rte->inh)
        {
                /* row-identity logic expects to add stuff to processed_tlist */
@@ -125,23 +126,15 @@ preprocess_targetlist(PlannerInfo *root)
        }
 
        /*
-        * For MERGE we need to handle the target list for the target relation,
-        * and also target list for each action (only INSERT/UPDATE matter).
+        * For MERGE we also need to handle the target list for each INSERT and
+        * UPDATE action separately.  In addition, we examine the qual of each
+        * action and add any Vars there (other than those of the target rel) to
+        * the subplan targetlist.
         */
        if (command_type == CMD_MERGE)
        {
                ListCell   *l;
 
-               /*
-                * For MERGE, add any junk column(s) needed to allow the executor to
-                * identify the rows to be inserted or updated.
-                */
-               root->processed_tlist = tlist;
-               add_row_identity_columns(root, result_relation,
-                                                                target_rte, target_relation);
-
-               tlist = root->processed_tlist;
-
                /*
                 * For MERGE, handle targetlist of each MergeAction separately. Give
                 * the same treatment to MergeAction->targetList as we would have
@@ -151,6 +144,8 @@ preprocess_targetlist(PlannerInfo *root)
                foreach(l, parse->mergeActionList)
                {
                        MergeAction *action = (MergeAction *) lfirst(l);
+                       List       *vars;
+                       ListCell   *l2;
 
                        if (action->commandType == CMD_INSERT)
                                action->targetList = expand_insert_targetlist(action->targetList,
@@ -158,6 +153,36 @@ preprocess_targetlist(PlannerInfo *root)
                        else if (action->commandType == CMD_UPDATE)
                                action->updateColnos =
                                        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.)
+                        */
+                       vars = pull_var_clause((Node *)
+                                                                  list_concat_copy((List *) action->qual,
+                                                                                                       action->targetList),
+                                                                  0);
+                       foreach(l2, vars)
+                       {
+                               Var                *var = (Var *) lfirst(l2);
+                               TargetEntry *tle;
+
+                               if (IsA(var, Var) && var->varno == result_relation)
+                                       continue;       /* don't need it */
+
+                               if (tlist_member((Expr *) var, tlist))
+                                       continue;       /* already got it */
+
+                               tle = makeTargetEntry((Expr *) var,
+                                                                         list_length(tlist) + 1,
+                                                                         NULL, true);
+                               tlist = lappend(tlist, tle);
+                       }
+                       list_free(vars);
                }
        }
 
index 5d0035a12b621eb7ecbd40fd46d7a688dcd0a3d3..bb9d76306b74f382bfe7c7cf1fe0b8e00499b572 100644 (file)
@@ -18,7 +18,6 @@
 #include "access/sysattr.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
-#include "nodes/nodeFuncs.h"
 #include "parser/analyze.h"
 #include "parser/parse_collate.h"
 #include "parser/parsetree.h"
@@ -205,9 +204,11 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt)
                                           pstate->p_target_nsitem->p_names->aliasname),
                                errdetail("The name is used both as MERGE target table and data source."));
 
-       qry->targetList = expandNSItemAttrs(pstate, nsitem, 0, false,
-                                                                               exprLocation(stmt->sourceRelation));
-
+       /*
+        * There's no need for a targetlist here; it'll be set up by
+        * preprocess_targetlist later.
+        */
+       qry->targetList = NIL;
        qry->rtable = pstate->p_rtable;
 
        /*
index 5954f10b8ff8016debfa8be4ae041beaa3562307..0fd037b45a8a2959c4173098be151b3cccf9c2e5 100644 (file)
@@ -791,6 +791,19 @@ SELECT * FROM wq_target;
    1 |     299
 (1 row)
 
+-- check source-side whole-row references
+BEGIN;
+MERGE INTO wq_target t
+USING wq_source s ON (t.tid = s.sid)
+WHEN matched and t = s or t.tid = s.sid THEN
+       UPDATE SET balance = t.balance + s.balance;
+SELECT * FROM wq_target;
+ tid | balance 
+-----+---------
+   1 |     399
+(1 row)
+
+ROLLBACK;
 -- check if subqueries work in the conditions?
 MERGE INTO wq_target t
 USING wq_source s ON t.tid = s.sid
index 7c6de7cc07c5ec4cbaa54d42a35341d295a02e08..57e145b0f5a3886157a378a1795836797be17d46 100644 (file)
@@ -2800,7 +2800,7 @@ WHEN NOT MATCHED THEN INSERT VALUES(o.k, o.v);
      ->  Result
            Output: 1, 'cte_basic val'::text
    ->  Hash Right Join
-         Output: (0), ('merge source SubPlan'::text), m.ctid
+         Output: m.ctid, (0), ('merge source SubPlan'::text)
          Hash Cond: (m.k = (0))
          ->  Seq Scan on public.m
                Output: m.ctid, m.k
@@ -2847,7 +2847,7 @@ WHEN NOT MATCHED THEN INSERT VALUES(o.k, o.v);
                  Output: (cte_init.b || ' merge update'::text)
                  Filter: (cte_init.a = 1)
    ->  Hash Right Join
-         Output: (1), ('merge source InitPlan'::text), m.ctid
+         Output: m.ctid, (1), ('merge source InitPlan'::text)
          Hash Cond: (m.k = (1))
          ->  Seq Scan on public.m
                Output: m.ctid, m.k
@@ -2889,7 +2889,7 @@ WHEN NOT MATCHED THEN INSERT VALUES(o.a, o.b || (SELECT merge_source_cte.*::text
      ->  CTE Scan on merge_source_cte merge_source_cte_2
            Output: ((merge_source_cte_2.*)::text || ' merge insert'::text)
    ->  Hash Right Join
-         Output: merge_source_cte.a, merge_source_cte.b, m.ctid
+         Output: m.ctid, merge_source_cte.a, merge_source_cte.b
          Hash Cond: (m.k = merge_source_cte.a)
          ->  Seq Scan on public.m
                Output: m.ctid, m.k
index 6d05a2f39ca654fcaf81760e49562698996c2f62..8815e0cc4982b89c7af577e14c17b5553b2a9e4d 100644 (file)
@@ -527,6 +527,15 @@ WHEN MATCHED AND t.balance = 199 OR s.balance > 100 THEN
        UPDATE SET balance = t.balance + s.balance;
 SELECT * FROM wq_target;
 
+-- check source-side whole-row references
+BEGIN;
+MERGE INTO wq_target t
+USING wq_source s ON (t.tid = s.sid)
+WHEN matched and t = s or t.tid = s.sid THEN
+       UPDATE SET balance = t.balance + s.balance;
+SELECT * FROM wq_target;
+ROLLBACK;
+
 -- check if subqueries work in the conditions?
 MERGE INTO wq_target t
 USING wq_source s ON t.tid = s.sid