pg.ddx.io pgsql-bugs@postgresql.org mailing list archive help / color / mirror / Atom feed From: Andres Freund <andres@anarazel.de>
Subject: [PATCH v1] heavily-WIP: partition specific subplan plans
Date: Wed, 22 Feb 2023 11:03:02 -0800
---
src/include/executor/execExpr.h | 1 +
src/include/nodes/execnodes.h | 17 +++++++
src/include/nodes/params.h | 2 +-
src/backend/executor/execExpr.c | 15 ++++++
src/backend/executor/execExprInterp.c | 33 +++++++++++--
src/backend/executor/execParallel.c | 2 +-
src/backend/executor/execPartition.c | 12 +++++
src/backend/executor/execUtils.c | 1 +
src/backend/executor/nodeCtescan.c | 2 +-
src/backend/executor/nodeRecursiveunion.c | 2 +-
src/backend/executor/nodeSubplan.c | 57 +++++++++++++++++------
src/backend/executor/nodeWorktablescan.c | 2 +-
12 files changed, 122 insertions(+), 24 deletions(-)
inline patch 1: src/include/executor/execExpr.h, src/include/nodes/execnodes.h, src/include/nodes/params.h and 9 more (+122/-24 lines) download inline-1.patch diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 86e1ac1e65c..6d2f66bbb93 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -379,6 +379,7 @@ typedef struct ExprEvalStep
{
int paramid; /* numeric ID for parameter */
Oid paramtype; /* OID of parameter's datatype */
+ int paramthing; /* offset into EState->es_param_exec_plans[paramid][] */
} param;
/* for EEOP_PARAM_CALLBACK */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 20f4c8b35f3..252c8ba7820 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -650,6 +650,21 @@ typedef struct EState
ParamListInfo es_param_list_info; /* values of external params */
ParamExecData *es_param_exec_vals; /* values of internal params */
+ /*
+ * two level list: es_param_exec_plans[paramid][target relation]
+ */
+ List *es_param_exec_plans;
+
+ /*
+ * Subplans may end up being partition specific. For that we need to have
+ * some sort of identifier to index into
+ * ->es_param_exec_plans[paramid]. That index needs to be available when
+ * building the expression. We don't have a good identifier for all
+ * partitions, so we assign these sequentially.
+ */
+ int es_param_exec_target_thing_current;
+ int es_param_exec_target_thing_next;
+
QueryEnvironment *es_queryEnv; /* query environment */
/* Other working state: */
@@ -972,6 +987,8 @@ typedef struct SubPlanState
FmgrInfo *lhs_hash_funcs; /* hash functions for lefthand datatype(s) */
FmgrInfo *cur_eq_funcs; /* equality functions for LHS vs. table */
ExprState *cur_eq_comp; /* equality comparator for LHS vs. table */
+ /* identifies offset in Estate->es_param_exec_plans[paramid][offset] */
+ int target_thing;
} SubPlanState;
/*
diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h
index ad23113a61c..f78bdf14268 100644
--- a/src/include/nodes/params.h
+++ b/src/include/nodes/params.h
@@ -145,7 +145,7 @@ typedef struct ParamListInfoData
typedef struct ParamExecData
{
- void *execPlan; /* should be "SubPlanState *" */
+ bool needsExecPlan; /* plan is in EState->es_param_exec_plans */
Datum value;
bool isnull;
} ParamExecData;
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 812ead95bc6..74a6b774bd0 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -993,6 +993,21 @@ ExecInitExprRec(Expr *node, ExprState *state,
scratch.opcode = EEOP_PARAM_EXEC;
scratch.d.param.paramid = param->paramid;
scratch.d.param.paramtype = param->paramtype;
+
+ /*
+ * If we're currently building an expression for a
+ * partition, a subplan might be specific to the
+ * current target partition. Unfortunately we can't
+ * tell yet, because we've not necessarily encountered
+ * the subplan yet. Need to decide at expression
+ * evaluation time.
+ */
+ if (!state->parent)
+ scratch.d.param.paramthing = 0;
+ else
+ scratch.d.param.paramthing =
+ state->parent->state->es_param_exec_target_thing_current;
+
ExprEvalPushStep(state, &scratch);
break;
case PARAM_EXTERN:
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 19351fe34bf..7a6d6bf4579 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -2420,14 +2420,39 @@ void
ExecEvalParamExec(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
{
ParamExecData *prm;
+ int paramid = op->d.param.paramid;
- prm = &(econtext->ecxt_param_exec_vals[op->d.param.paramid]);
- if (unlikely(prm->execPlan != NULL))
+ prm = &(econtext->ecxt_param_exec_vals[paramid]);
+
+ if (unlikely(prm->needsExecPlan))
{
+ List *param_list;
+ EState *estate = econtext->ecxt_estate;
+ SubPlanState *param_plan;
+
+ Assert(list_length(estate->es_param_exec_plans) >= paramid + 1);
+ param_list = list_nth(estate->es_param_exec_plans, paramid);
+
+ /*
+ * The plan could either be specific to the partitioned table, or
+ * not. Unfortunately it looks to be hard to determine that in
+ * execExpr.c. So we just try both - there shouldn't ever be
+ * conflicts.
+ */
+ if (list_length(param_list) >= op->d.param.paramthing + 1)
+ {
+ param_plan = castNode(SubPlanState, list_nth(param_list, op->d.param.paramthing));
+ }
+ else
+ {
+ Assert(list_length(param_list) == 1);
+ param_plan = linitial(param_list);
+ }
+
/* Parameter not evaluated yet, so go do it */
- ExecSetParamPlan(prm->execPlan, econtext);
+ ExecSetParamPlan(param_plan, econtext);
/* ExecSetParamPlan should have processed this param... */
- Assert(prm->execPlan == NULL);
+ Assert(!prm->needsExecPlan);
}
*op->resvalue = prm->value;
*op->resnull = prm->isnull;
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index aa3f2834537..2e868425e3f 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -429,7 +429,7 @@ RestoreParamExecParams(char *start_address, EState *estate)
/* Read datum/isnull. */
prm->value = datumRestore(&start_address, &prm->isnull);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
}
}
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 651ad24fc10..e93494c7e18 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -510,9 +510,19 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
MemoryContext oldcxt;
AttrMap *part_attmap = NULL;
bool found_whole_row;
+ size_t save_target_thing_current = estate->es_param_exec_target_thing_current;
oldcxt = MemoryContextSwitchTo(proute->memcxt);
+ /*
+ * We can't use partidx or such as an identifier for partition specific
+ * state, due to multi-level partitioning. So we just assign the
+ * identifier sequentially. One advantage of that scheme is that we end up
+ * with far smaller indexes when partition pruning is effective.
+ */
+ Assert(estate->es_param_exec_target_thing_next != 0);
+ estate->es_param_exec_target_thing_current = estate->es_param_exec_target_thing_next++;
+
partrel = table_open(partOid, RowExclusiveLock);
leaf_part_rri = makeNode(ResultRelInfo);
@@ -959,6 +969,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
}
MemoryContextSwitchTo(oldcxt);
+ estate->es_param_exec_target_thing_current = save_target_thing_current;
+
return leaf_part_rri;
}
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index c33a3c0bec7..fef21a1953b 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -138,6 +138,7 @@ CreateExecutorState(void)
estate->es_param_list_info = NULL;
estate->es_param_exec_vals = NULL;
+ estate->es_param_exec_target_thing_next = 1;
estate->es_queryEnv = NULL;
diff --git a/src/backend/executor/nodeCtescan.c b/src/backend/executor/nodeCtescan.c
index cc4c4243e2f..b6b59410791 100644
--- a/src/backend/executor/nodeCtescan.c
+++ b/src/backend/executor/nodeCtescan.c
@@ -224,7 +224,7 @@ ExecInitCteScan(CteScan *node, EState *estate, int eflags)
* CTEs, particularly the shared tuplestore.
*/
prmdata = &(estate->es_param_exec_vals[node->cteParam]);
- Assert(prmdata->execPlan == NULL);
+ Assert(!prmdata->needsExecPlan);
Assert(!prmdata->isnull);
scanstate->leader = castNode(CteScanState, DatumGetPointer(prmdata->value));
if (scanstate->leader == NULL)
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index e7810039341..bafbdf4e2d9 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -215,7 +215,7 @@ ExecInitRecursiveUnion(RecursiveUnion *node, EState *estate, int eflags)
* via the Param slot reserved for it.
*/
prmdata = &(estate->es_param_exec_vals[node->wtParam]);
- Assert(prmdata->execPlan == NULL);
+ Assert(!prmdata->needsExecPlan);
prmdata->value = PointerGetDatum(rustate);
prmdata->isnull = false;
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index 64af36a1873..16c0998a65d 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -270,7 +270,7 @@ ExecScanSubPlan(SubPlanState *node,
int paramid = lfirst_int(l);
ParamExecData *prm = &(estate->es_param_exec_vals[paramid]);
- prm->execPlan = node;
+ prm->needsExecPlan = true;
}
*isNull = true;
return (Datum) 0;
@@ -413,7 +413,7 @@ ExecScanSubPlan(SubPlanState *node,
ParamExecData *prmdata;
prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
- Assert(prmdata->execPlan == NULL);
+ Assert(!prmdata->needsExecPlan);
prmdata->value = slot_getattr(slot, col, &(prmdata->isnull));
col++;
}
@@ -598,7 +598,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
ParamExecData *prmdata;
prmdata = &(innerecontext->ecxt_param_exec_vals[paramid]);
- Assert(prmdata->execPlan == NULL);
+ Assert(!prmdata->needsExecPlan);
prmdata->value = slot_getattr(slot, col,
&(prmdata->isnull));
col++;
@@ -846,6 +846,7 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
sstate->tab_collations = NULL;
sstate->lhs_hash_funcs = NULL;
sstate->cur_eq_funcs = NULL;
+ sstate->target_thing = estate->es_param_exec_target_thing_current;
/*
* If this is an initplan or MULTIEXPR subplan, it has output parameters
@@ -867,8 +868,21 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
{
int paramid = lfirst_int(lst);
ParamExecData *prm = &(estate->es_param_exec_vals[paramid]);
+ ListCell *param_cell;
- prm->execPlan = sstate;
+ prm->needsExecPlan = true;
+
+ while (list_length(estate->es_param_exec_plans) <= paramid)
+ estate->es_param_exec_plans = lappend(estate->es_param_exec_plans, NULL);
+
+ param_cell = list_nth_cell(estate->es_param_exec_plans, paramid);
+
+ while (list_length(lfirst(param_cell)) <= sstate->target_thing)
+ lfirst(param_cell) = lappend(lfirst(param_cell), NULL);
+
+ if (list_nth(lfirst(param_cell), sstate->target_thing) != NULL)
+ elog(WARNING, "overwriting previous subplan exec plan: %u", sstate->target_thing);
+ lfirst(list_nth_cell(lfirst(param_cell), sstate->target_thing)) = sstate;
}
}
@@ -1143,7 +1157,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
int paramid = linitial_int(subplan->setParam);
ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
prm->value = BoolGetDatum(true);
prm->isnull = false;
found = true;
@@ -1193,7 +1207,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
int paramid = lfirst_int(l);
ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
prm->value = heap_getattr(node->curTuple, i, tdesc,
&(prm->isnull));
i++;
@@ -1216,7 +1230,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
node->curArray = makeArrayResultAny(astate,
econtext->ecxt_per_query_memory,
true);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
prm->value = node->curArray;
prm->isnull = false;
}
@@ -1228,7 +1242,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
int paramid = linitial_int(subplan->setParam);
ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
prm->value = BoolGetDatum(false);
prm->isnull = false;
}
@@ -1240,7 +1254,7 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
int paramid = lfirst_int(l);
ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
- prm->execPlan = NULL;
+ prm->needsExecPlan = false;
prm->value = (Datum) 0;
prm->isnull = true;
}
@@ -1268,18 +1282,31 @@ void
ExecSetParamPlanMulti(const Bitmapset *params, ExprContext *econtext)
{
int paramid;
+ EState *estate = econtext->ecxt_estate;
paramid = -1;
while ((paramid = bms_next_member(params, paramid)) >= 0)
{
ParamExecData *prm = &(econtext->ecxt_param_exec_vals[paramid]);
- if (prm->execPlan != NULL)
+ if (prm->needsExecPlan)
{
- /* Parameter not evaluated yet, so go do it */
- ExecSetParamPlan(prm->execPlan, econtext);
- /* ExecSetParamPlan should have processed this param... */
- Assert(prm->execPlan == NULL);
+ ListCell *lc;
+ List *param_list;
+
+ Assert(list_length(estate->es_param_exec_plans) >= paramid);
+ param_list = list_nth(estate->es_param_exec_plans, paramid);
+
+ foreach(lc, param_list)
+ {
+ if (lfirst(lc) == NULL)
+ continue;
+
+ /* Parameter not evaluated yet, so go do it */
+ ExecSetParamPlan(lfirst(lc), econtext);
+ /* ExecSetParamPlan should have processed this param... */
+ Assert(!prm->needsExecPlan);
+ }
}
}
}
@@ -1321,7 +1348,7 @@ ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent)
ParamExecData *prm = &(estate->es_param_exec_vals[paramid]);
if (subplan->subLinkType != CTE_SUBLINK)
- prm->execPlan = node;
+ prm->needsExecPlan = node;
parent->chgParam = bms_add_member(parent->chgParam, paramid);
}
diff --git a/src/backend/executor/nodeWorktablescan.c b/src/backend/executor/nodeWorktablescan.c
index 0c13448236a..4202ab74955 100644
--- a/src/backend/executor/nodeWorktablescan.c
+++ b/src/backend/executor/nodeWorktablescan.c
@@ -95,7 +95,7 @@ ExecWorkTableScan(PlanState *pstate)
ParamExecData *param;
param = &(estate->es_param_exec_vals[plan->wtParam]);
- Assert(param->execPlan == NULL);
+ Assert(!param->needsExecPlan);
Assert(!param->isnull);
node->rustate = castNode(RecursiveUnionState, DatumGetPointer(param->value));
Assert(node->rustate);
--
2.38.0
Message-ID: <no-message-id-28018@localhost>
Permalink: ../no-message-id-28018@localhost/
Also on: postgresql.org/message-id/no-message-id-28018@localhost
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: andres@anarazel.de
Subject: Re: [PATCH v1] heavily-WIP: partition specific subplan plans
In-Reply-To: <no-message-id-28018@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox