Re: BUG #17800: ON CONFLICT DO UPDATE fails to detect incompatible fields that leads to a server crash - Mailing list pgsql-bugs

OK, so this worked out quite well ... it's only about 50 net new lines
of code, and there's no data structure changes outside the contents of
compiled expressions, so no reason to fear ABI problems.

I did the renaming you had comments suggesting, but perhaps you want
to bikeshed those names?

            regards, tom lane

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 812ead95bc..05d56fb5b2 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -52,12 +52,15 @@
 #include "utils/typcache.h"


-typedef struct LastAttnumInfo
+typedef struct ExprPreScanInfo
 {
+    /* Highest attribute numbers fetched from inner/outer/scan tuple slots: */
     AttrNumber    last_inner;
     AttrNumber    last_outer;
     AttrNumber    last_scan;
-} LastAttnumInfo;
+    /* MULTIEXPR SubPlan nodes appearing in the expression: */
+    List       *multiexpr_subplans;
+} ExprPreScanInfo;

 static void ExecReadyExpr(ExprState *state);
 static void ExecInitExprRec(Expr *node, ExprState *state,
@@ -65,9 +68,9 @@ static void ExecInitExprRec(Expr *node, ExprState *state,
 static void ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args,
                          Oid funcid, Oid inputcollid,
                          ExprState *state);
-static void ExecInitExprSlots(ExprState *state, Node *node);
-static void ExecPushExprSlots(ExprState *state, LastAttnumInfo *info);
-static bool get_last_attnums_walker(Node *node, LastAttnumInfo *info);
+static void ExecCreateSetupSteps(ExprState *state, Node *node);
+static void ExecPushSetupSteps(ExprState *state, ExprPreScanInfo *info);
+static bool expr_pre_scan_walker(Node *node, ExprPreScanInfo *info);
 static bool ExecComputeSlotInfo(ExprState *state, ExprEvalStep *op);
 static void ExecInitWholeRowVar(ExprEvalStep *scratch, Var *variable,
                                 ExprState *state);
@@ -136,8 +139,8 @@ ExecInitExpr(Expr *node, PlanState *parent)
     state->parent = parent;
     state->ext_params = NULL;

-    /* Insert EEOP_*_FETCHSOME steps as needed */
-    ExecInitExprSlots(state, (Node *) node);
+    /* Insert setup steps as needed */
+    ExecCreateSetupSteps(state, (Node *) node);

     /* Compile the expression proper */
     ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
@@ -173,8 +176,8 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
     state->parent = NULL;
     state->ext_params = ext_params;

-    /* Insert EEOP_*_FETCHSOME steps as needed */
-    ExecInitExprSlots(state, (Node *) node);
+    /* Insert setup steps as needed */
+    ExecCreateSetupSteps(state, (Node *) node);

     /* Compile the expression proper */
     ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
@@ -228,8 +231,8 @@ ExecInitQual(List *qual, PlanState *parent)
     /* mark expression as to be used with ExecQual() */
     state->flags = EEO_FLAG_IS_QUAL;

-    /* Insert EEOP_*_FETCHSOME steps as needed */
-    ExecInitExprSlots(state, (Node *) qual);
+    /* Insert setup steps as needed */
+    ExecCreateSetupSteps(state, (Node *) qual);

     /*
      * ExecQual() needs to return false for an expression returning NULL. That
@@ -372,8 +375,8 @@ ExecBuildProjectionInfo(List *targetList,

     state->resultslot = slot;

-    /* Insert EEOP_*_FETCHSOME steps as needed */
-    ExecInitExprSlots(state, (Node *) targetList);
+    /* Insert setup steps as needed */
+    ExecCreateSetupSteps(state, (Node *) targetList);

     /* Now compile each tlist column */
     foreach(lc, targetList)
@@ -524,7 +527,7 @@ ExecBuildUpdateProjection(List *targetList,
     int            nAssignableCols;
     bool        sawJunk;
     Bitmapset  *assignedCols;
-    LastAttnumInfo deform = {0, 0, 0};
+    ExprPreScanInfo deform = {0, 0, 0, NIL};
     ExprEvalStep scratch = {0};
     int            outerattnum;
     ListCell   *lc,
@@ -603,11 +606,11 @@ ExecBuildUpdateProjection(List *targetList,
      * number of columns of the "outer" tuple.
      */
     if (evalTargetList)
-        get_last_attnums_walker((Node *) targetList, &deform);
+        expr_pre_scan_walker((Node *) targetList, &deform);
     else
         deform.last_outer = nAssignableCols;

-    ExecPushExprSlots(state, &deform);
+    ExecPushSetupSteps(state, &deform);

     /*
      * Now generate code to evaluate the tlist's assignable expressions or
@@ -677,20 +680,8 @@ ExecBuildUpdateProjection(List *targetList,
     }

     /*
-     * If we're evaluating the tlist, must evaluate any resjunk columns too.
-     * (This matters for things like MULTIEXPR_SUBLINK SubPlans.)
+     * We don't bother evaluating any tlist entries that are marked resjunk.
      */
-    if (evalTargetList)
-    {
-        for_each_cell(lc, targetList, lc)
-        {
-            TargetEntry *tle = lfirst_node(TargetEntry, lc);
-
-            Assert(tle->resjunk);
-            ExecInitExprRec(tle->expr, state,
-                            &state->resvalue, &state->resnull);
-        }
-    }

     /*
      * Now generate code to copy over any old columns that were not assigned
@@ -1402,6 +1393,21 @@ ExecInitExprRec(Expr *node, ExprState *state,
                 SubPlan    *subplan = (SubPlan *) node;
                 SubPlanState *sstate;

+                /*
+                 * Real execution of a MULTIEXPR SubPlan has already been
+                 * done. What we have to do here is return a dummy NULL record
+                 * value in case this targetlist element is assigned
+                 * someplace.
+                 */
+                if (subplan->subLinkType == MULTIEXPR_SUBLINK)
+                {
+                    scratch.opcode = EEOP_CONST;
+                    scratch.d.constval.value = (Datum) 0;
+                    scratch.d.constval.isnull = true;
+                    ExprEvalPushStep(state, &scratch);
+                    break;
+                }
+
                 if (!state->parent)
                     elog(ERROR, "SubPlan found with no parent plan");

@@ -2542,36 +2548,38 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
 }

 /*
- * Add expression steps deforming the ExprState's inner/outer/scan slots
- * as much as required by the expression.
+ * Add expression steps performing setup that's needed before any of the
+ * main execution of the expression.
  */
 static void
-ExecInitExprSlots(ExprState *state, Node *node)
+ExecCreateSetupSteps(ExprState *state, Node *node)
 {
-    LastAttnumInfo info = {0, 0, 0};
+    ExprPreScanInfo info = {0, 0, 0, NIL};

-    /*
-     * Figure out which attributes we're going to need.
-     */
-    get_last_attnums_walker(node, &info);
+    /* Prescan to find out what we need. */
+    expr_pre_scan_walker(node, &info);

-    ExecPushExprSlots(state, &info);
+    /* And generate those steps. */
+    ExecPushSetupSteps(state, &info);
 }

 /*
- * Add steps deforming the ExprState's inner/out/scan slots as much as
- * indicated by info. This is useful when building an ExprState covering more
- * than one expression.
+ * Add steps performing expression setup as indicated by "info".
+ * This is useful when building an ExprState covering more than one expression.
  */
 static void
-ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
+ExecPushSetupSteps(ExprState *state, ExprPreScanInfo *info)
 {
     ExprEvalStep scratch = {0};
+    ListCell   *lc;

     scratch.resvalue = NULL;
     scratch.resnull = NULL;

-    /* Emit steps as needed */
+    /*
+     * Add steps deforming the ExprState's inner/outer/scan slots as much as
+     * required by any Vars appearing in the expression.
+     */
     if (info->last_inner > 0)
     {
         scratch.opcode = EEOP_INNER_FETCHSOME;
@@ -2602,13 +2610,48 @@ ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
         if (ExecComputeSlotInfo(state, &scratch))
             ExprEvalPushStep(state, &scratch);
     }
+
+    /*
+     * Add steps to execute any MULTIEXPR SubPlans appearing in the
+     * expression.  We need to evaluate these before any of the Params
+     * referencing their outputs are used, but after we've prepared for any
+     * Var references they may contain.  (There cannot be cross-references
+     * between MULTIEXPR SubPlans, so we needn't worry about their order.)
+     */
+    foreach(lc, info->multiexpr_subplans)
+    {
+        SubPlan    *subplan = (SubPlan *) lfirst(lc);
+        SubPlanState *sstate;
+
+        Assert(subplan->subLinkType == MULTIEXPR_SUBLINK);
+
+        /* This should match what ExecInitExprRec does for other SubPlans: */
+
+        if (!state->parent)
+            elog(ERROR, "SubPlan found with no parent plan");
+
+        sstate = ExecInitSubPlan(subplan, state->parent);
+
+        /* add SubPlanState nodes to state->parent->subPlan */
+        state->parent->subPlan = lappend(state->parent->subPlan,
+                                         sstate);
+
+        scratch.opcode = EEOP_SUBPLAN;
+        scratch.d.subplan.sstate = sstate;
+
+        /* The result can be ignored, but we better put it somewhere */
+        scratch.resvalue = &state->resvalue;
+        scratch.resnull = &state->resnull;
+
+        ExprEvalPushStep(state, &scratch);
+    }
 }

 /*
- * get_last_attnums_walker: expression walker for ExecInitExprSlots
+ * expr_pre_scan_walker: expression walker for ExecCreateSetupSteps
  */
 static bool
-get_last_attnums_walker(Node *node, LastAttnumInfo *info)
+expr_pre_scan_walker(Node *node, ExprPreScanInfo *info)
 {
     if (node == NULL)
         return false;
@@ -2636,6 +2679,16 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
         return false;
     }

+    /* Collect all MULTIEXPR SubPlans, too */
+    if (IsA(node, SubPlan))
+    {
+        SubPlan    *subplan = (SubPlan *) node;
+
+        if (subplan->subLinkType == MULTIEXPR_SUBLINK)
+            info->multiexpr_subplans = lappend(info->multiexpr_subplans,
+                                               subplan);
+    }
+
     /*
      * Don't examine the arguments or filters of Aggrefs or WindowFuncs,
      * because those do not represent expressions to be evaluated within the
@@ -2648,7 +2701,7 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
         return false;
     if (IsA(node, GroupingFunc))
         return false;
-    return expression_tree_walker(node, get_last_attnums_walker,
+    return expression_tree_walker(node, expr_pre_scan_walker,
                                   (void *) info);
 }

@@ -3267,7 +3320,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
     PlanState  *parent = &aggstate->ss.ps;
     ExprEvalStep scratch = {0};
     bool        isCombine = DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
-    LastAttnumInfo deform = {0, 0, 0};
+    ExprPreScanInfo deform = {0, 0, 0, NIL};

     state->expr = (Expr *) aggstate;
     state->parent = parent;
@@ -3283,18 +3336,18 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
     {
         AggStatePerTrans pertrans = &aggstate->pertrans[transno];

-        get_last_attnums_walker((Node *) pertrans->aggref->aggdirectargs,
-                                &deform);
-        get_last_attnums_walker((Node *) pertrans->aggref->args,
-                                &deform);
-        get_last_attnums_walker((Node *) pertrans->aggref->aggorder,
-                                &deform);
-        get_last_attnums_walker((Node *) pertrans->aggref->aggdistinct,
-                                &deform);
-        get_last_attnums_walker((Node *) pertrans->aggref->aggfilter,
-                                &deform);
+        expr_pre_scan_walker((Node *) pertrans->aggref->aggdirectargs,
+                             &deform);
+        expr_pre_scan_walker((Node *) pertrans->aggref->args,
+                             &deform);
+        expr_pre_scan_walker((Node *) pertrans->aggref->aggorder,
+                             &deform);
+        expr_pre_scan_walker((Node *) pertrans->aggref->aggdistinct,
+                             &deform);
+        expr_pre_scan_walker((Node *) pertrans->aggref->aggfilter,
+                             &deform);
     }
-    ExecPushExprSlots(state, &deform);
+    ExecPushSetupSteps(state, &deform);

     /*
      * Emit instructions for each transition value / grouping set combination.
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 64af36a187..c136f75ac2 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -235,47 +235,6 @@ ExecScanSubPlan(SubPlanState *node,
     ListCell   *l;
     ArrayBuildStateAny *astate = NULL;

-    /*
-     * MULTIEXPR subplans, when "executed", just return NULL; but first we
-     * mark the subplan's output parameters as needing recalculation.  (This
-     * is a bit of a hack: it relies on the subplan appearing later in its
-     * targetlist than any of the referencing Params, so that all the Params
-     * have been evaluated before we re-mark them for the next evaluation
-     * cycle.  But in general resjunk tlist items appear after non-resjunk
-     * ones, so this should be safe.)  Unlike ExecReScanSetParamPlan, we do
-     * *not* set bits in the parent plan node's chgParam, because we don't
-     * want to cause a rescan of the parent.
-     *
-     * Note: we are also relying on MULTIEXPR SubPlans not sharing any output
-     * parameters with other SubPlans, because if one does then it is unclear
-     * which SubPlanState node the parameter's execPlan field will be pointing
-     * to when we come to evaluate the parameter.  We can allow plain initplan
-     * SubPlans to share output parameters, because it doesn't actually matter
-     * which initplan SubPlan we reference as long as they all point to the
-     * same underlying subplan.  However, that fails to hold for MULTIEXPRs
-     * because they can have non-empty args lists, and the "same" args might
-     * have mutated into different forms in different parts of a plan tree.
-     * There is currently no problem because MULTIEXPR can appear only in an
-     * UPDATE's top-level target list, so it won't get duplicated anyplace.
-     * Postgres versions before v14 had to make concrete efforts to avoid
-     * sharing output parameters across different clones of a MULTIEXPR, and
-     * the problem could recur someday.
-     */
-    if (subLinkType == MULTIEXPR_SUBLINK)
-    {
-        EState       *estate = node->parent->state;
-
-        foreach(l, subplan->setParam)
-        {
-            int            paramid = lfirst_int(l);
-            ParamExecData *prm = &(estate->es_param_exec_vals[paramid]);
-
-            prm->execPlan = node;
-        }
-        *isNull = true;
-        return (Datum) 0;
-    }
-
     /* Initialize ArrayBuildStateAny in caller's context, if needed */
     if (subLinkType == ARRAY_SUBLINK)
         astate = initArrayResultAny(subplan->firstColType,
@@ -327,6 +286,11 @@ ExecScanSubPlan(SubPlanState *node,
      * NULL.  Assuming we get a tuple, we just use its first column (there can
      * be only one non-junk column in this case).
      *
+     * For MULTIEXPR_SUBLINK, we push the per-column subplan outputs out to
+     * the setParams and then return a dummy false value.  There must not be
+     * multiple tuples returned from the subplan; if zero tuples are produced,
+     * set the setParams to NULL.
+     *
      * For ARRAY_SUBLINK we allow the subplan to produce any number of tuples,
      * and form an array of the first column's values.  Note in particular
      * that we produce a zero-element array if no tuples are produced (this is
@@ -378,6 +342,47 @@ ExecScanSubPlan(SubPlanState *node,
             continue;
         }

+        if (subLinkType == MULTIEXPR_SUBLINK)
+        {
+            /* cannot allow multiple input tuples for MULTIEXPR sublink */
+            if (found)
+                ereport(ERROR,
+                        (errcode(ERRCODE_CARDINALITY_VIOLATION),
+                         errmsg("more than one row returned by a subquery used as an expression")));
+            found = true;
+
+            /*
+             * We need to copy the subplan's tuple in case any result is of
+             * pass-by-ref type --- our output values will point into this
+             * copied tuple!  Can't use the subplan's instance of the tuple
+             * since it won't still be valid after next ExecProcNode() call.
+             * node->curTuple keeps track of the copied tuple for eventual
+             * freeing.
+             */
+            if (node->curTuple)
+                heap_freetuple(node->curTuple);
+            node->curTuple = ExecCopySlotHeapTuple(slot);
+
+            /*
+             * Now set all the setParam params from the columns of the tuple
+             */
+            col = 1;
+            foreach(plst, subplan->setParam)
+            {
+                int            paramid = lfirst_int(plst);
+                ParamExecData *prmdata;
+
+                prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
+                Assert(prmdata->execPlan == NULL);
+                prmdata->value = heap_getattr(node->curTuple, col, tdesc,
+                                              &(prmdata->isnull));
+                col++;
+            }
+
+            /* keep scanning subplan to make sure there's only one tuple */
+            continue;
+        }
+
         if (subLinkType == ARRAY_SUBLINK)
         {
             Datum        dvalue;
@@ -473,6 +478,20 @@ ExecScanSubPlan(SubPlanState *node,
             result = (Datum) 0;
             *isNull = true;
         }
+        else if (subLinkType == MULTIEXPR_SUBLINK)
+        {
+            /* We don't care about function result, but set the setParams */
+            foreach(l, subplan->setParam)
+            {
+                int            paramid = lfirst_int(l);
+                ParamExecData *prmdata;
+
+                prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
+                Assert(prmdata->execPlan == NULL);
+                prmdata->value = (Datum) 0;
+                prmdata->isnull = true;
+            }
+        }
     }

     return result;
@@ -848,10 +867,9 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
     sstate->cur_eq_funcs = NULL;

     /*
-     * If this is an initplan or MULTIEXPR subplan, it has output parameters
-     * that the parent plan will use, so mark those parameters as needing
-     * evaluation.  We don't actually run the subplan until we first need one
-     * of its outputs.
+     * If this is an initplan, it has output parameters that the parent plan
+     * will use, so mark those parameters as needing evaluation.  We don't
+     * actually run the subplan until we first need one of its outputs.
      *
      * A CTE subplan's output parameter is never to be evaluated in the normal
      * way, so skip this in that case.
@@ -859,7 +877,8 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
      * Note that we don't set parent->chgParam here: the parent plan hasn't
      * been run yet, so no need to force it to re-run.
      */
-    if (subplan->setParam != NIL && subplan->subLinkType != CTE_SUBLINK)
+    if (subplan->setParam != NIL && subplan->parParam == NIL &&
+        subplan->subLinkType != CTE_SUBLINK)
     {
         ListCell   *lst;

@@ -1079,7 +1098,6 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
     ScanDirection dir = estate->es_direction;
     MemoryContext oldcontext;
     TupleTableSlot *slot;
-    ListCell   *pvar;
     ListCell   *l;
     bool        found = false;
     ArrayBuildStateAny *astate = NULL;
@@ -1089,6 +1107,8 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
         elog(ERROR, "ANY/ALL subselect unsupported as initplan");
     if (subLinkType == CTE_SUBLINK)
         elog(ERROR, "CTE subplans should not be executed via ExecSetParamPlan");
+    if (subplan->parParam || node->args)
+        elog(ERROR, "correlated subplans should not be executed via ExecSetParamPlan");

     /*
      * Enforce forward scan direction regardless of caller. It's hard but not
@@ -1106,26 +1126,6 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
      */
     oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory);

-    /*
-     * Set Params of this plan from parent plan correlation values. (Any
-     * calculation we have to do is done in the parent econtext, since the
-     * Param values don't need to have per-query lifetime.)  Currently, we
-     * expect only MULTIEXPR_SUBLINK plans to have any correlation values.
-     */
-    Assert(subplan->parParam == NIL || subLinkType == MULTIEXPR_SUBLINK);
-    Assert(list_length(subplan->parParam) == list_length(node->args));
-
-    forboth(l, subplan->parParam, pvar, node->args)
-    {
-        int            paramid = lfirst_int(l);
-        ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
-
-        prm->value = ExecEvalExprSwitchContext((ExprState *) lfirst(pvar),
-                                               econtext,
-                                               &(prm->isnull));
-        planstate->chgParam = bms_add_member(planstate->chgParam, paramid);
-    }
-
     /*
      * Run the plan.  (If it needs to be rescanned, the first ExecProcNode
      * call will take care of that.)
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 1be1642d92..b4292253cc 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -972,9 +972,9 @@ typedef struct SubLink
  * The values are assigned to the global PARAM_EXEC params indexed by parParam
  * (the parParam and args lists must have the same ordering).  setParam is a
  * list of the PARAM_EXEC params that are computed by the sub-select, if it
- * is an initplan; they are listed in order by sub-select output column
- * position.  (parParam and setParam are integer Lists, not Bitmapsets,
- * because their ordering is significant.)
+ * is an initplan or MULTIEXPR plan; they are listed in order by sub-select
+ * output column position.  (parParam and setParam are integer Lists, not
+ * Bitmapsets, because their ordering is significant.)
  *
  * Also, the planner computes startup and per-call costs for use of the
  * SubPlan.  Note that these include the cost of the subquery proper,
@@ -1009,8 +1009,8 @@ typedef struct SubPlan
     /* Note: parallel_safe does not consider contents of testexpr or args */
     /* Information for passing params into and out of the subselect: */
     /* setParam and parParam are lists of integers (param IDs) */
-    List       *setParam;        /* initplan subqueries have to set these
-                                 * Params for parent plan */
+    List       *setParam;        /* initplan and MULTIEXPR subqueries have to
+                                 * set these Params for parent plan */
     List       *parParam;        /* indices of input Params from parent plan */
     List       *args;            /* exprs to pass as parParam values */
     /* Estimated execution costs: */
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2d49e765de..e2a0dc80b2 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1713,6 +1713,116 @@ reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;
 --
+-- Check handling of MULTIEXPR SubPlans in inherited updates
+--
+create table inhpar(f1 int, f2 name);
+create table inhcld(f2 name, f1 int);
+alter table inhcld inherit inhpar;
+insert into inhpar select x, x::text from generate_series(1,5) x;
+insert into inhcld select x::text, x from generate_series(6,10) x;
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+                               QUERY PLAN
+-------------------------------------------------------------------------
+ Update on public.inhpar i
+   Update on public.inhpar i_1
+   Update on public.inhcld i_2
+   ->  Result
+         Output: $2, $3, (SubPlan 1 (returns $2,$3)), i.tableoid, i.ctid
+         ->  Append
+               ->  Seq Scan on public.inhpar i_1
+                     Output: i_1.f1, i_1.f2, i_1.tableoid, i_1.ctid
+               ->  Seq Scan on public.inhcld i_2
+                     Output: i_2.f1, i_2.f2, i_2.tableoid, i_2.ctid
+         SubPlan 1 (returns $2,$3)
+           ->  Limit
+                 Output: (i.f1), (((i.f2)::text || '-'::text))
+                 ->  Seq Scan on public.int4_tbl
+                       Output: i.f1, ((i.f2)::text || '-'::text)
+(15 rows)
+
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+ f1 | f2
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+drop table inhpar cascade;
+NOTICE:  drop cascades to table inhcld
+--
+-- And the same for partitioned cases
+--
+create table inhpar(f1 int primary key, f2 name) partition by range (f1);
+create table inhcld1(f2 name, f1 int primary key);
+create table inhcld2(f1 int primary key, f2 name);
+alter table inhpar attach partition inhcld1 for values from (1) to (5);
+alter table inhpar attach partition inhcld2 for values from (5) to (100);
+insert into inhpar select x, x::text from generate_series(1,10) x;
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+                                    QUERY PLAN
+-----------------------------------------------------------------------------------
+ Update on public.inhpar i
+   Update on public.inhcld1 i_1
+   Update on public.inhcld2 i_2
+   ->  Append
+         ->  Seq Scan on public.inhcld1 i_1
+               Output: $2, $3, (SubPlan 1 (returns $2,$3)), i_1.tableoid, i_1.ctid
+               SubPlan 1 (returns $2,$3)
+                 ->  Limit
+                       Output: (i_1.f1), (((i_1.f2)::text || '-'::text))
+                       ->  Seq Scan on public.int4_tbl
+                             Output: i_1.f1, ((i_1.f2)::text || '-'::text)
+         ->  Seq Scan on public.inhcld2 i_2
+               Output: $2, $3, (SubPlan 1 (returns $2,$3)), i_2.tableoid, i_2.ctid
+(13 rows)
+
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+ f1 | f2
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+-- Also check ON CONFLICT
+insert into inhpar as i values (3), (7) on conflict (f1)
+  do update set (f1, f2) = (select i.f1, i.f2 || '+');
+select * from inhpar order by f1;  -- tuple order might be unstable here
+ f1 | f2
+----+-----
+  1 | 1-
+  2 | 2-
+  3 | 3-+
+  4 | 4-
+  5 | 5-
+  6 | 6-
+  7 | 7-+
+  8 | 8-
+  9 | 9-
+ 10 | 10-
+(10 rows)
+
+drop table inhpar cascade;
+--
 -- Check handling of a constant-null CHECK constraint
 --
 create table cnullparent (f1 int);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 195aedb5ff..5db6dbc191 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -629,6 +629,44 @@ reset enable_seqscan;
 reset enable_indexscan;
 reset enable_bitmapscan;

+--
+-- Check handling of MULTIEXPR SubPlans in inherited updates
+--
+create table inhpar(f1 int, f2 name);
+create table inhcld(f2 name, f1 int);
+alter table inhcld inherit inhpar;
+insert into inhpar select x, x::text from generate_series(1,5) x;
+insert into inhcld select x::text, x from generate_series(6,10) x;
+
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+
+drop table inhpar cascade;
+
+--
+-- And the same for partitioned cases
+--
+create table inhpar(f1 int primary key, f2 name) partition by range (f1);
+create table inhcld1(f2 name, f1 int primary key);
+create table inhcld2(f1 int primary key, f2 name);
+alter table inhpar attach partition inhcld1 for values from (1) to (5);
+alter table inhpar attach partition inhcld2 for values from (5) to (100);
+insert into inhpar select x, x::text from generate_series(1,10) x;
+
+explain (verbose, costs off)
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+update inhpar i set (f1, f2) = (select i.f1, i.f2 || '-' from int4_tbl limit 1);
+select * from inhpar;
+
+-- Also check ON CONFLICT
+insert into inhpar as i values (3), (7) on conflict (f1)
+  do update set (f1, f2) = (select i.f1, i.f2 || '+');
+select * from inhpar order by f1;  -- tuple order might be unstable here
+
+drop table inhpar cascade;
+
 --
 -- Check handling of a constant-null CHECK constraint
 --