public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v18 3/5] rename archive-modules.sgml to archive-and-restore-modules.sgml
11+ messages / 6 participants
[nested] [flat]
* [PATCH v18 3/5] rename archive-modules.sgml to archive-and-restore-modules.sgml
@ 2023-02-15 22:45 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Nathan Bossart @ 2023-02-15 22:45 UTC (permalink / raw)
---
.../{archive-modules.sgml => archive-and-restore-modules.sgml} | 0
doc/src/sgml/filelist.sgml | 2 +-
doc/src/sgml/postgres.sgml | 2 +-
3 files changed, 2 insertions(+), 2 deletions(-)
rename doc/src/sgml/{archive-modules.sgml => archive-and-restore-modules.sgml} (100%)
diff --git a/doc/src/sgml/archive-modules.sgml b/doc/src/sgml/archive-and-restore-modules.sgml
similarity index 100%
rename from doc/src/sgml/archive-modules.sgml
rename to doc/src/sgml/archive-and-restore-modules.sgml
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index e0dca81cb2..31a57943f7 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -101,7 +101,7 @@
<!ENTITY custom-scan SYSTEM "custom-scan.sgml">
<!ENTITY logicaldecoding SYSTEM "logicaldecoding.sgml">
<!ENTITY replication-origins SYSTEM "replication-origins.sgml">
-<!ENTITY archive-modules SYSTEM "archive-modules.sgml">
+<!ENTITY archive-and-restore-modules SYSTEM "archive-and-restore-modules.sgml">
<!ENTITY protocol SYSTEM "protocol.sgml">
<!ENTITY sources SYSTEM "sources.sgml">
<!ENTITY storage SYSTEM "storage.sgml">
diff --git a/doc/src/sgml/postgres.sgml b/doc/src/sgml/postgres.sgml
index 2c107199d3..23522ccaf3 100644
--- a/doc/src/sgml/postgres.sgml
+++ b/doc/src/sgml/postgres.sgml
@@ -228,7 +228,7 @@ break is not needed in a wider output rendering.
&bgworker;
&logicaldecoding;
&replication-origins;
- &archive-modules;
+ &archive-and-restore-modules;
</part>
--
2.25.1
--IJpNTDwzlM2Ie8A6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v18-0004-restructure-archive-modules-docs-in-preparation-.patch"
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v1] WIP: Evaluate arguments of correlated SubPlans in the referencing ExprState
@ 2023-02-25 21:39 Andres Freund <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2023-02-25 21:39 UTC (permalink / raw)
---
src/include/nodes/execnodes.h | 1 -
src/backend/executor/execExpr.c | 81 +++++++++++++++++------------
src/backend/executor/execProcnode.c | 5 ++
src/backend/executor/nodeSubplan.c | 30 +++++------
4 files changed, 66 insertions(+), 51 deletions(-)
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 20f4c8b35f3..437cf8b5a02 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -947,7 +947,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: */
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index c61f23c6c18..7a9d5729b4b 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -87,6 +87,9 @@ static void ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
int transno, int setno, int setoff, bool ishash,
bool nullcheck);
+static void ExecInitSubPlanExpr(SubPlan *subplan,
+ ExprState *state,
+ Datum *resv, bool *resnull);
/*
@@ -1388,7 +1391,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
case T_SubPlan:
{
SubPlan *subplan = (SubPlan *) node;
- SubPlanState *sstate;
/*
* Real execution of a MULTIEXPR SubPlan has already been
@@ -1405,19 +1407,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;
}
@@ -2618,29 +2608,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);
}
}
@@ -4040,3 +4013,45 @@ ExecBuildParamSetEqual(TupleDesc desc,
return state;
}
+
+static void
+ExecInitSubPlanExpr(SubPlan *subplan,
+ ExprState *state,
+ Datum *resv, bool *resnull)
+{
+ ExprEvalStep scratch = {0};
+ SubPlanState *sstate;
+ ListCell *pvar;
+ ListCell *l;
+ EState *estate = state->parent->state;
+
+ if (!state->parent)
+ elog(ERROR, "SubPlan found with no parent plan");
+
+ /*
+ * Generate steps to evaluate input arguments for the subplan.
+ *
+ * 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.
+ */
+ forboth(l, subplan->parParam, pvar, subplan->args)
+ {
+ int paramid = lfirst_int(l);
+ ParamExecData *prm = &estate->es_param_exec_vals[paramid];
+
+ ExecInitExprRec(lfirst(pvar), state, &prm->value, &prm->isnull);
+ }
+
+ 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);
+}
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index 4d288bc8d41..f62bb28140f 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -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);
}
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index c136f75ac24..3458ac007cd 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -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,20 @@ 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)
+ Assert(list_length(subplan->parParam) == list_length(subplan->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 +810,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 +841,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 +1103,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");
/*
--
2.38.0
--6ir4gcdcv6d4j3qy--
^ permalink raw reply [nested|flat] 11+ messages in thread
* [PATCH v2] WIP: Evaluate arguments of correlated SubPlans in the referencing ExprState
@ 2023-02-25 21:39 Andres Freund <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Andres Freund @ 2023-02-25 21:39 UTC (permalink / raw)
---
src/include/executor/execExpr.h | 6 +-
src/include/nodes/execnodes.h | 1 -
src/backend/executor/execExpr.c | 93 +++++++++++++++++----------
src/backend/executor/execExprInterp.c | 22 +++++++
src/backend/executor/execProcnode.c | 5 ++
src/backend/executor/nodeSubplan.c | 30 ++++-----
src/backend/jit/llvm/llvmjit_expr.c | 6 ++
src/backend/jit/llvm/llvmjit_types.c | 1 +
8 files changed, 112 insertions(+), 52 deletions(-)
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 06c3adc0a19..ca2b7306cd0 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -158,6 +158,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,
@@ -374,7 +376,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 */
@@ -738,6 +740,8 @@ extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
+extern void ExecEvalParamSet(ExprState *state, ExprEvalStep *op,
+ ExprContext *econtext);
extern void ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op);
extern void ExecEvalNextValueExpr(ExprState *state, ExprEvalStep *op);
extern void ExecEvalRowNull(ExprState *state, ExprEvalStep *op,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e7eb1e666ff..16e95e4cb48 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -947,7 +947,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: */
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index c61f23c6c18..002f2a0091f 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -87,6 +87,9 @@ static void ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
FunctionCallInfo fcinfo, AggStatePerTrans pertrans,
int transno, int setno, int setoff, bool ishash,
bool nullcheck);
+static void ExecInitSubPlanExpr(SubPlan *subplan,
+ ExprState *state,
+ Datum *resv, bool *resnull);
/*
@@ -1388,7 +1391,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
case T_SubPlan:
{
SubPlan *subplan = (SubPlan *) node;
- SubPlanState *sstate;
/*
* Real execution of a MULTIEXPR SubPlan has already been
@@ -1405,19 +1407,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;
}
@@ -2618,29 +2608,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);
}
}
@@ -4040,3 +4013,57 @@ ExecBuildParamSetEqual(TupleDesc desc,
return state;
}
+
+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.
+ */
+ forboth(l, subplan->parParam, pvar, subplan->args)
+ {
+ int paramid = lfirst_int(l);
+
+ ExecInitExprRec(lfirst(pvar), state,
+ &state->resvalue, &state->resnull);
+
+ scratch.opcode = EEOP_PARAM_SET;
+ scratch.d.param.paramid = paramid;
+ /* type isn't needed, but an old value could be confusing */
+ scratch.d.param.paramtype = InvalidOid;
+ 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);
+}
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 19351fe34bf..3cab8a5cdae 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -446,6 +446,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,
@@ -1081,6 +1082,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)
{
/*
@@ -2477,6 +2485,20 @@ 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]);
+ prm->value = state->resvalue;
+ prm->isnull = state->resnull;
+}
+
/*
* Raise error if a CURRENT OF expression is evaluated.
*
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index 4d288bc8d41..f62bb28140f 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -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);
}
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index c136f75ac24..3458ac007cd 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -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,20 @@ 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)
+ Assert(list_length(subplan->parParam) == list_length(subplan->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 +810,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 +841,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 +1103,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");
/*
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 1c722c79552..812f8758743 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1094,6 +1094,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;
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index 876fb640294..0b1df3afe00 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -123,6 +123,7 @@ void *referenced_functions[] =
ExecEvalNextValueExpr,
ExecEvalParamExec,
ExecEvalParamExtern,
+ ExecEvalParamSet,
ExecEvalRow,
ExecEvalRowNotNull,
ExecEvalRowNull,
--
2.37.1.188.g71a8fab31b
--2fo66m5ddj2bowyf--
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-16 14:19 torikoshia <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: torikoshia @ 2025-04-16 14:19 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; [email protected]; +Cc: pgsql-hackers
On 2025-04-16 06:18, Daniel Gustafsson wrote:
>> On 15 Apr 2025, at 23:03, David Rowley <[email protected]> wrote:
>
>> My vote is to make the levels 1-based in all locations where we output
>> the context information.
>
> I agree with this, pg_get_process_memory_contexts() also use 1-based
> levels
> fwiw.
+1.
I believe there's no particular issue with starting the level from 1 in
pg_log_backend_memory_contexts().
Regarding the implementation:
In the initial patch attached, I naïvely incremented the level just
before emitting the log line.
However, it might be cleaner to simply initialize the level variable to
1 from the start. This could help avoid unnecessary confusion when
debugging that part of the code.
Similarly, I noticed that in pg_get_process_memory_contexts(), the level
is initialized to 0 in ProcessGetMemoryContextInterrupt(void):
int level = 0;
..
MemoryContextStatsInternal(c, level, 100, 100, &grand_totals, ..
If we want to be consistent, perhaps it would make sense to start from 1
there as well.
BTW level variable has existed since before pg_backend_memory_contexts
was introduced — it was originally used for functions that help inspect
memory contexts via the debugger. Because of that, I think changing this
would affect not only these functions codes but some older ones.
--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-17 12:25 Rahila Syed <[email protected]>
parent: torikoshia <[email protected]>
0 siblings, 2 replies; 11+ messages in thread
From: Rahila Syed @ 2025-04-17 12:25 UTC (permalink / raw)
To: pgsql-hackers
Hi,
> The attached patch is how I think we should do it.
>
Thank you for the patch.
I tested this patch and it works fine. I agree with the changes made in it.
Regarding v2 patch,
- int level = 0;
Retaining the level variable will enhance the code readability, IMO.
Thank you,
Rahila Syed
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-17 14:27 torikoshia <[email protected]>
parent: Rahila Syed <[email protected]>
1 sibling, 0 replies; 11+ messages in thread
From: torikoshia @ 2025-04-17 14:27 UTC (permalink / raw)
To: Rahila Syed <[email protected]>; [email protected]; +Cc: pgsql-hackers
Thanks for your review, Melih and Rahila.
On 2025-04-17 21:25, Rahila Syed wrote:
> Hi,
>
>> The attached patch is how I think we should do it.
>
> Thank you for the patch.
> I tested this patch and it works fine. I agree with the changes made
> in it.
>
> Regarding v2 patch,
> - int level = 0;
>
> Retaining the level variable will enhance the code readability, IMO.
As for the level variable, this change comes from the v1 patch, and I
don't have a strong opinion about it.
However, if we decide to keep the level variable here, it might be more
consistent to also define it in MemoryContextStatsDetail().
--
Regards,
--
Atsushi Torikoshi
Seconded from NTT DATA GROUP CORPORATION to SRA OSS K.K.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-17 21:11 David Rowley <[email protected]>
parent: Rahila Syed <[email protected]>
1 sibling, 1 reply; 11+ messages in thread
From: David Rowley @ 2025-04-17 21:11 UTC (permalink / raw)
To: Rahila Syed <[email protected]>; +Cc: pgsql-hackers
On Fri, 18 Apr 2025 at 00:25, Rahila Syed <[email protected]> wrote:
> Regarding v2 patch,
> - int level = 0;
>
> Retaining the level variable will enhance the code readability, IMO.
When I read that, I suspected it might have been leftover from a
refactor during the development that was forgotten about. There'd be
thousands of places in our code base that you could make the
readability argument for, including the max_level and max_children
parameters at the same call-site. But those didn't get the same
treatment.
I've now pushed the latest patch. Thanks for the reviews.
David
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-18 08:54 Fujii Masao <[email protected]>
parent: David Rowley <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Fujii Masao @ 2025-04-18 08:54 UTC (permalink / raw)
To: David Rowley <[email protected]>; Rahila Syed <[email protected]>; +Cc: pgsql-hackers
On 2025/04/18 6:11, David Rowley wrote:
> On Fri, 18 Apr 2025 at 00:25, Rahila Syed <[email protected]> wrote:
>> Regarding v2 patch,
>> - int level = 0;
>>
>> Retaining the level variable will enhance the code readability, IMO.
>
> When I read that, I suspected it might have been leftover from a
> refactor during the development that was forgotten about. There'd be
> thousands of places in our code base that you could make the
> readability argument for, including the max_level and max_children
> parameters at the same call-site. But those didn't get the same
> treatment.
>
> I've now pushed the latest patch. Thanks for the reviews.
Shouldn't the example output of pg_log_backend_memory_contexts() in
the documentation also be updated to use 1-based numbering for consistency?
Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 7b3aa9de467005e540b2246ed89c9e43fe08f6bd Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 18 Apr 2025 17:45:29 +0900
Subject: [PATCH v1] doc: Fix memory context level in
pg_log_backend_memory_contexts() example.
Commit d9e03864b6b changed the memory context level numbers shown by
pg_log_backend_memory_contexts() to be 1-based. However, the example in
the documentation was not updated and still used 0-based numbering.
This commit updates the example to match the current 1-based output.
---
doc/src/sgml/func.sgml | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1c5cfee25d1..574a544d9fa 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -28922,16 +28922,16 @@ One message for each memory context will be logged. For example:
<screen>
LOG: logging memory contexts of PID 10377
STATEMENT: SELECT pg_log_backend_memory_contexts(pg_backend_pid());
-LOG: level: 0; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
-LOG: level: 1; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
-LOG: level: 1; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
-LOG: level: 1; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
-LOG: level: 1; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
-LOG: level: 1; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
-LOG: level: 1; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
-LOG: level: 1; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
+LOG: level: 1; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
+LOG: level: 2; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
+LOG: level: 2; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
+LOG: level: 2; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
+LOG: level: 2; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
+LOG: level: 2; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
+LOG: level: 2; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
+LOG: level: 2; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
...
-LOG: level: 1; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
+LOG: level: 2; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
LOG: Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560 used
</screen>
If there are more than 100 child contexts under the same parent, the first
--
2.49.0
Attachments:
[text/plain] v1-0001-doc-Fix-memory-context-level-in-pg_log_backend_me.patch (2.9K, ../../[email protected]/2-v1-0001-doc-Fix-memory-context-level-in-pg_log_backend_me.patch)
download | inline diff:
From 7b3aa9de467005e540b2246ed89c9e43fe08f6bd Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 18 Apr 2025 17:45:29 +0900
Subject: [PATCH v1] doc: Fix memory context level in
pg_log_backend_memory_contexts() example.
Commit d9e03864b6b changed the memory context level numbers shown by
pg_log_backend_memory_contexts() to be 1-based. However, the example in
the documentation was not updated and still used 0-based numbering.
This commit updates the example to match the current 1-based output.
---
doc/src/sgml/func.sgml | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 1c5cfee25d1..574a544d9fa 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -28922,16 +28922,16 @@ One message for each memory context will be logged. For example:
<screen>
LOG: logging memory contexts of PID 10377
STATEMENT: SELECT pg_log_backend_memory_contexts(pg_backend_pid());
-LOG: level: 0; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
-LOG: level: 1; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
-LOG: level: 1; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
-LOG: level: 1; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
-LOG: level: 1; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
-LOG: level: 1; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
-LOG: level: 1; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
-LOG: level: 1; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
+LOG: level: 1; TopMemoryContext: 80800 total in 6 blocks; 14432 free (5 chunks); 66368 used
+LOG: level: 2; pgstat TabStatusArray lookup hash table: 8192 total in 1 blocks; 1408 free (0 chunks); 6784 used
+LOG: level: 2; TopTransactionContext: 8192 total in 1 blocks; 7720 free (1 chunks); 472 used
+LOG: level: 2; RowDescriptionContext: 8192 total in 1 blocks; 6880 free (0 chunks); 1312 used
+LOG: level: 2; MessageContext: 16384 total in 2 blocks; 5152 free (0 chunks); 11232 used
+LOG: level: 2; Operator class cache: 8192 total in 1 blocks; 512 free (0 chunks); 7680 used
+LOG: level: 2; smgr relation table: 16384 total in 2 blocks; 4544 free (3 chunks); 11840 used
+LOG: level: 2; TransactionAbortContext: 32768 total in 1 blocks; 32504 free (0 chunks); 264 used
...
-LOG: level: 1; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
+LOG: level: 2; ErrorContext: 8192 total in 1 blocks; 7928 free (3 chunks); 264 used
LOG: Grand total: 1651920 bytes in 201 blocks; 622360 free (88 chunks); 1029560 used
</screen>
If there are more than 100 child contexts under the same parent, the first
--
2.49.0
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-18 09:23 David Rowley <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: David Rowley @ 2025-04-18 09:23 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Rahila Syed <[email protected]>; pgsql-hackers
On Fri, 18 Apr 2025 at 20:54, Fujii Masao <[email protected]> wrote:
> Shouldn't the example output of pg_log_backend_memory_contexts() in
> the documentation also be updated to use 1-based numbering for consistency?
> Patch attached.
Yeah. I failed to notice we had an example of the output.
Want to take care of it?
David
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-18 09:45 Fujii Masao <[email protected]>
parent: David Rowley <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Fujii Masao @ 2025-04-18 09:45 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Rahila Syed <[email protected]>; pgsql-hackers
On 2025/04/18 18:23, David Rowley wrote:
> On Fri, 18 Apr 2025 at 20:54, Fujii Masao <[email protected]> wrote:
>> Shouldn't the example output of pg_log_backend_memory_contexts() in
>> the documentation also be updated to use 1-based numbering for consistency?
>> Patch attached.
>
> Yeah. I failed to notice we had an example of the output.
>
> Want to take care of it?
Yeah, I will if you're okay with that!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Align memory context level numbering in pg_log_backend_memory_contexts()
@ 2025-04-21 05:59 Fujii Masao <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Fujii Masao @ 2025-04-21 05:59 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Rahila Syed <[email protected]>; pgsql-hackers
On 2025/04/18 18:45, Fujii Masao wrote:
>
>
> On 2025/04/18 18:23, David Rowley wrote:
>> On Fri, 18 Apr 2025 at 20:54, Fujii Masao <[email protected]> wrote:
>>> Shouldn't the example output of pg_log_backend_memory_contexts() in
>>> the documentation also be updated to use 1-based numbering for consistency?
>>> Patch attached.
>>
>> Yeah. I failed to notice we had an example of the output.
>>
>> Want to take care of it?
>
> Yeah, I will if you're okay with that!
Pushed. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2025-04-21 05:59 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-02-15 22:45 [PATCH v18 3/5] rename archive-modules.sgml to archive-and-restore-modules.sgml Nathan Bossart <[email protected]>
2023-02-25 21:39 [PATCH v2] WIP: Evaluate arguments of correlated SubPlans in the referencing ExprState Andres Freund <[email protected]>
2023-02-25 21:39 [PATCH v1] WIP: Evaluate arguments of correlated SubPlans in the referencing ExprState Andres Freund <[email protected]>
2025-04-16 14:19 Re: Align memory context level numbering in pg_log_backend_memory_contexts() torikoshia <[email protected]>
2025-04-17 12:25 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() Rahila Syed <[email protected]>
2025-04-17 14:27 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() torikoshia <[email protected]>
2025-04-17 21:11 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() David Rowley <[email protected]>
2025-04-18 08:54 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() Fujii Masao <[email protected]>
2025-04-18 09:23 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() David Rowley <[email protected]>
2025-04-18 09:45 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() Fujii Masao <[email protected]>
2025-04-21 05:59 ` Re: Align memory context level numbering in pg_log_backend_memory_contexts() Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox