Evaluate arguments of correlated SubPlans in the referencing ExprState
authorAndres Freund <[email protected]>
Thu, 1 Aug 2024 01:11:49 +0000 (18:11 -0700)
committerAndres Freund <[email protected]>
Thu, 1 Aug 2024 02:54:46 +0000 (19:54 -0700)
Until now we generated an ExprState for each parameter to a SubPlan and
evaluated them one-by-one ExecScanSubPlan. That's sub-optimal as creating lots
of small ExprStates
a) makes JIT compilation more expensive
b) wastes memory
c) is a bit slower to execute

This commit arranges to evaluate parameters to a SubPlan as part of the
ExprState referencing a SubPlan, using the new EEOP_PARAM_SET expression
step. We emit one EEOP_PARAM_SET for each argument to a subplan, just before
the EEOP_SUBPLAN step.

It likely is worth using EEOP_PARAM_SET in other places as well, e.g. for
SubPlan outputs, nestloop parameters and - more ambitiously - to get rid of
ExprContext->domainValue/caseValue/ecxt_agg*.  But that's for later.

Author: Andres Freund <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Reviewed-by: Alena Rybakina <[email protected]>
Discussion: https://postgr.es/m/20230225214401[email protected]

src/backend/executor/execExpr.c
src/backend/executor/execExprInterp.c
src/backend/executor/execProcnode.c
src/backend/executor/nodeSubplan.c
src/backend/jit/llvm/llvmjit_expr.c
src/backend/jit/llvm/llvmjit_types.c
src/include/executor/execExpr.h
src/include/nodes/execnodes.h

index 9b52bab52fcb0fc03078f07f7cd589be42957365..66dda8e5e6937f6844b43c11f7752d29c73bd37b 100644 (file)
@@ -69,6 +69,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 ExecInitSubPlanExpr(SubPlan *subplan,
+                               ExprState *state,
+                               Datum *resv, bool *resnull);
 static void ExecCreateExprSetupSteps(ExprState *state, Node *node);
 static void ExecPushExprSetupSteps(ExprState *state, ExprSetupInfo *info);
 static bool expr_setup_walker(Node *node, ExprSetupInfo *info);
@@ -1406,7 +1409,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
        case T_SubPlan:
            {
                SubPlan    *subplan = (SubPlan *) node;
-               SubPlanState *sstate;
 
                /*
                 * Real execution of a MULTIEXPR SubPlan has already been
@@ -1423,19 +1425,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
                    break;
                }
 
-               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;
-
-               ExprEvalPushStep(state, &scratch);
+               ExecInitSubPlanExpr(subplan, state, resv, resnull);
                break;
            }
 
@@ -2715,6 +2705,70 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
    }
 }
 
+/*
+ * Append the steps necessary for the evaluation of a SubPlan node to
+ * ExprState->steps.
+ *
+ * subplan - SubPlan expression to evaluate
+ * state - ExprState to whose ->steps to append the necessary operations
+ * resv / resnull - where to store the result of the node into
+ */
+static void
+ExecInitSubPlanExpr(SubPlan *subplan,
+                   ExprState *state,
+                   Datum *resv, bool *resnull)
+{
+   ExprEvalStep scratch = {0};
+   SubPlanState *sstate;
+   ListCell   *pvar;
+   ListCell   *l;
+
+   if (!state->parent)
+       elog(ERROR, "SubPlan found with no parent plan");
+
+   /*
+    * Generate steps to evaluate input arguments for the subplan.
+    *
+    * We evaluate the argument expressions into ExprState's resvalue/resnull,
+    * and then use PARAM_SET to update the parameter. We do that, instead of
+    * evaluating directly into the param, to avoid depending on the pointer
+    * value remaining stable / being included in the generated expression. No
+    * danger of conflicts with other uses of resvalue/resnull as storing and
+    * using the value always is in subsequent steps.
+    *
+    * Any calculation we have to do can be done in the parent econtext, since
+    * the Param values don't need to have per-query lifetime.
+    */
+   Assert(list_length(subplan->parParam) == list_length(subplan->args));
+   forboth(l, subplan->parParam, pvar, subplan->args)
+   {
+       int         paramid = lfirst_int(l);
+       Expr       *arg = (Expr *) lfirst(pvar);
+
+       ExecInitExprRec(arg, state,
+                       &state->resvalue, &state->resnull);
+
+       scratch.opcode = EEOP_PARAM_SET;
+       scratch.d.param.paramid = paramid;
+       /* paramtype's not actually used, but we might as well fill it */
+       scratch.d.param.paramtype = exprType((Node *) arg);
+       ExprEvalPushStep(state, &scratch);
+   }
+
+   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.resvalue = resv;
+   scratch.resnull = resnull;
+   scratch.d.subplan.sstate = sstate;
+
+   ExprEvalPushStep(state, &scratch);
+}
+
 /*
  * Add expression steps performing setup that's needed before any of the
  * main execution of the expression.
@@ -2789,29 +2843,12 @@ ExecPushExprSetupSteps(ExprState *state, ExprSetupInfo *info)
    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);
+       ExecInitSubPlanExpr(subplan, state,
+                           &state->resvalue, &state->resnull);
    }
 }
 
index 1633ea8373161627366f030ef4825216c69112da..1535fd6b984519945ff26d3f532d9aca1f650227 100644 (file)
@@ -450,6 +450,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
        &&CASE_EEOP_PARAM_EXEC,
        &&CASE_EEOP_PARAM_EXTERN,
        &&CASE_EEOP_PARAM_CALLBACK,
+       &&CASE_EEOP_PARAM_SET,
        &&CASE_EEOP_CASE_TESTVAL,
        &&CASE_EEOP_MAKE_READONLY,
        &&CASE_EEOP_IOCOERCE,
@@ -1093,6 +1094,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
            EEO_NEXT();
        }
 
+       EEO_CASE(EEOP_PARAM_SET)
+       {
+           /* out of line, unlikely to matter performancewise */
+           ExecEvalParamSet(state, op, econtext);
+           EEO_NEXT();
+       }
+
        EEO_CASE(EEOP_CASE_TESTVAL)
        {
            /*
@@ -2555,6 +2563,24 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
             errmsg("no value found for parameter %d", paramId)));
 }
 
+/*
+ * Set value of a param (currently always PARAM_EXEC) from
+ * state->res{value,null}.
+ */
+void
+ExecEvalParamSet(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
+{
+   ParamExecData *prm;
+
+   prm = &(econtext->ecxt_param_exec_vals[op->d.param.paramid]);
+
+   /* Shouldn't have a pending evaluation anymore */
+   Assert(prm->execPlan == NULL);
+
+   prm->value = state->resvalue;
+   prm->isnull = state->resnull;
+}
+
 /*
  * Evaluate a CoerceViaIO node in soft-error mode.
  *
index 6e48062f56a7ae82da95d1f3faa614487484e179..34f28dfeceae51eeeb8157e96bf0851b4dba43d7 100644 (file)
@@ -393,6 +393,10 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
    /*
     * Initialize any initPlans present in this node.  The planner put them in
     * a separate list for us.
+    *
+    * The defining characteristic of initplans is that they don't have
+    * arguments, so we don't need to evaluate them (in contrast to
+    * ExecInitSubPlanExpr()).
     */
    subps = NIL;
    foreach(l, node->initPlan)
@@ -401,6 +405,7 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
        SubPlanState *sstate;
 
        Assert(IsA(subplan, SubPlan));
+       Assert(subplan->args == NIL);
        sstate = ExecInitSubPlan(subplan, result);
        subps = lappend(subps, sstate);
    }
index 9697b1f396d561eb0735c1503202bb2b4efb9ec0..a96cdd01e1b5e000fe0cd5b778aa5138106d35b1 100644 (file)
@@ -107,7 +107,7 @@ ExecHashSubPlan(SubPlanState *node,
    TupleTableSlot *slot;
 
    /* Shouldn't have any direct correlation Vars */
-   if (subplan->parParam != NIL || node->args != NIL)
+   if (subplan->parParam != NIL || subplan->args != NIL)
        elog(ERROR, "hashed subplan with direct correlation not supported");
 
    /*
@@ -231,7 +231,6 @@ ExecScanSubPlan(SubPlanState *node,
    TupleTableSlot *slot;
    Datum       result;
    bool        found = false;  /* true if got at least one subplan tuple */
-   ListCell   *pvar;
    ListCell   *l;
    ArrayBuildStateAny *astate = NULL;
 
@@ -248,26 +247,19 @@ ExecScanSubPlan(SubPlanState *node,
    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.)
+    * We rely on the caller to evaluate plan correlation values, if
+    * necessary. However we still need to record the fact that the values
+    * (might have) changed, otherwise the ExecReScan() below won't know that
+    * nodes need to be rescanned.
     */
-   Assert(list_length(subplan->parParam) == list_length(node->args));
-
-   forboth(l, subplan->parParam, pvar, node->args)
+   foreach(l, subplan->parParam)
    {
        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);
    }
 
-   /*
-    * Now that we've set up its parameters, we can reset the subplan.
-    */
+   /* with that done, we can reset the subplan */
    ExecReScan(planstate);
 
    /*
@@ -817,6 +809,10 @@ slotNoNulls(TupleTableSlot *slot)
  * as well as regular SubPlans.  Note that we don't link the SubPlan into
  * the parent's subPlan list, because that shouldn't happen for InitPlans.
  * Instead, ExecInitExpr() does that one part.
+ *
+ * We also rely on ExecInitExpr(), more precisely ExecInitSubPlanExpr(), to
+ * evaluate input parameters, as that allows them to be evaluated as part of
+ * the expression referencing the SubPlan.
  * ----------------------------------------------------------------
  */
 SubPlanState *
@@ -844,7 +840,6 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 
    /* Initialize subexpressions */
    sstate->testexpr = ExecInitExpr((Expr *) subplan->testexpr, parent);
-   sstate->args = ExecInitExprList(subplan->args, parent);
 
    /*
     * initialize my state
@@ -1107,7 +1102,7 @@ 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)
+   if (subplan->parParam || subplan->args)
        elog(ERROR, "correlated subplans should not be executed via ExecSetParamPlan");
 
    /*
index cbd9ed7cc4b65829bae9320dabb2b8973f0825f7..27f94f90070a4ef4803cd01464c1b7ef9184d2f7 100644 (file)
@@ -1145,6 +1145,12 @@ llvm_compile_expr(ExprState *state)
                    break;
                }
 
+           case EEOP_PARAM_SET:
+               build_EvalXFunc(b, mod, "ExecEvalParamSet",
+                               v_state, op, v_econtext);
+               LLVMBuildBr(b, opblocks[opno + 1]);
+               break;
+
            case EEOP_SBSREF_SUBSCRIPTS:
                {
                    int         jumpdone = op->d.sbsref_subscript.jumpdone;
index f93c383fd52c593e504a751230f395071aec3bb5..4a9e07701458b05ecf52dba10f0ed3394d1cdd43 100644 (file)
@@ -160,6 +160,7 @@ void       *referenced_functions[] =
    ExecEvalNextValueExpr,
    ExecEvalParamExec,
    ExecEvalParamExtern,
+   ExecEvalParamSet,
    ExecEvalRow,
    ExecEvalRowNotNull,
    ExecEvalRowNull,
index d983a9a7fed8f87ac7b924fc4f942c910bc98612..845f3422dea7829b20d38372853dd6a09d015aa5 100644 (file)
@@ -160,6 +160,8 @@ typedef enum ExprEvalOp
    EEOP_PARAM_EXEC,
    EEOP_PARAM_EXTERN,
    EEOP_PARAM_CALLBACK,
+   /* set PARAM_EXEC value */
+   EEOP_PARAM_SET,
 
    /* return CaseTestExpr value */
    EEOP_CASE_TESTVAL,
@@ -384,7 +386,7 @@ typedef struct ExprEvalStep
            ExprEvalRowtypeCache rowcache;
        }           nulltest_row;
 
-       /* for EEOP_PARAM_EXEC/EXTERN */
+       /* for EEOP_PARAM_EXEC/EXTERN and EEOP_PARAM_SET */
        struct
        {
            int         paramid;    /* numeric ID for parameter */
@@ -800,6 +802,8 @@ extern void ExecEvalFuncExprStrictFusage(ExprState *state, ExprEvalStep *op,
                                         ExprContext *econtext);
 extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op,
                              ExprContext *econtext);
+extern void ExecEvalParamSet(ExprState *state, ExprEvalStep *op,
+                            ExprContext *econtext);
 extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op,
                                ExprContext *econtext);
 extern void ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op);
index cac684d9b3af778474e1fec6e23ffa8354fe13c1..c3670f7158c8a3572c424d19155e403b5f337e5f 100644 (file)
@@ -961,7 +961,6 @@ typedef struct SubPlanState
    struct PlanState *planstate;    /* subselect plan's state tree */
    struct PlanState *parent;   /* parent plan node's state tree */
    ExprState  *testexpr;       /* state of combining expression */
-   List       *args;           /* states of argument expression(s) */
    HeapTuple   curTuple;       /* copy of most recent tuple from subplan */
    Datum       curArray;       /* most recent array from ARRAY() subplan */
    /* these are used when hashing the subselect's output: */