public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Zhihong Yu <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Thu, 1 Dec 2022 16:59:25 +0900
Message-ID: <CA+HiwqG=yLd843jneu09fG+hFTQWYYw1xtxchh0hA5N+gofJhg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+HiwqFXoR3kBHE-RhnQ2OGzKxXM03RJ_mDUrWLXXbRDcSgvDg@mail.gmail.com>
<[email protected]>
Hi Alvaro,
Thanks for looking at this one.
On Thu, Dec 1, 2022 at 3:12 AM Alvaro Herrera <[email protected]> wrote:
> Looking at 0001, I wonder if we should have a crosscheck that a
> PartitionPruneInfo you got from following an index is indeed constructed
> for the relation that you think it is: previously, you were always sure
> that the prune struct is for this node, because you followed a pointer
> that was set up in the node itself. Now you only have an index, and you
> have to trust that the index is correct.
Yeah, a crosscheck sounds like a good idea.
> I'm not sure how to implement this, or even if it's doable at all.
> Keeping the OID of the partitioned table in the PartitionPruneInfo
> struct is easy, but I don't know how to check it in ExecInitMergeAppend
> and ExecInitAppend.
Hmm, how about keeping the [Merge]Append's parent relation's RT index
in the PartitionPruneInfo and passing it down to
ExecInitPartitionPruning() from ExecInit[Merge]Append() for
cross-checking? Both Append and MergeAppend already have a
'apprelids' field that we can save a copy of in the
PartitionPruneInfo. Tried that in the attached delta patch.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] PartitionPruneInfo-relids.patch (5.3K, 2-PartitionPruneInfo-relids.patch)
download | inline diff:
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 2bd069d889..9a631a9192 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1791,6 +1791,9 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
* Initialize data structure needed for run-time partition pruning and
* do initial pruning if needed
*
+ * 'root_parent_relids' identifies the relation to which both the parent plan
+ * and the PartitionPruneInfo given by 'part_prune_index' belong.
+ *
* On return, *initially_valid_subplans is assigned the set of indexes of
* child subplans that must be initialized along with the parent plan node.
* Initial pruning is performed here if needed and in that case only the
@@ -1804,6 +1807,7 @@ PartitionPruneState *
ExecInitPartitionPruning(PlanState *planstate,
int n_total_subplans,
int part_prune_index,
+ Bitmapset *root_parent_relids,
Bitmapset **initially_valid_subplans)
{
PartitionPruneState *prunestate;
@@ -1811,6 +1815,14 @@ ExecInitPartitionPruning(PlanState *planstate,
PartitionPruneInfo *pruneinfo = list_nth(estate->es_part_prune_infos,
part_prune_index);
+ /* Sanity: part_prune_index gives the correct PartitionPruneInfo. */
+ if (!bms_equal(root_parent_relids, pruneinfo->root_parent_relids))
+ elog(ERROR, "wrong relids (%s) found in PartitionPruneInfo at part_prune_index=%u which has root_parent_relids=%s",
+ bmsToString(root_parent_relids),
+ part_prune_index,
+ bmsToString(pruneinfo->root_parent_relids));
+
+
/* We may need an expression context to evaluate partition exprs */
ExecAssignExprContext(estate, planstate);
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index c6f86a6510..99830198bd 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -146,6 +146,7 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
prunestate = ExecInitPartitionPruning(&appendstate->ps,
list_length(node->appendplans),
node->part_prune_index,
+ node->apprelids,
&validsubplans);
appendstate->as_prune_state = prunestate;
nplans = bms_num_members(validsubplans);
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 8d35860c30..f370f9f287 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -94,6 +94,7 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
prunestate = ExecInitPartitionPruning(&mergestate->ps,
list_length(node->mergeplans),
node->part_prune_index,
+ node->apprelids,
&validsubplans);
mergestate->ms_prune_state = prunestate;
nplans = bms_num_members(validsubplans);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 720f20f563..e67f0e3509 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -354,6 +354,8 @@ set_plan_references(PlannerInfo *root, Plan *plan)
PartitionPruneInfo *pruneinfo = lfirst(lc);
ListCell *l;
+ pruneinfo->root_parent_relids =
+ offset_relid_set(pruneinfo->root_parent_relids, rtoffset);
foreach(l, pruneinfo->prune_infos)
{
List *prune_infos = lfirst(l);
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 6565b6ed01..d48f6784c1 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -340,6 +340,7 @@ make_partition_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
/* Else build the result data structure */
pruneinfo = makeNode(PartitionPruneInfo);
+ pruneinfo->root_parent_relids = parentrel->relids;
pruneinfo->prune_infos = prunerelinfos;
/*
diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h
index bf962af7af..17fabc18c9 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -124,6 +124,7 @@ typedef struct PartitionPruneState
extern PartitionPruneState *ExecInitPartitionPruning(PlanState *planstate,
int n_total_subplans,
int part_prune_index,
+ Bitmapset *root_parent_relids,
Bitmapset **initially_valid_subplans);
extern Bitmapset *ExecFindMatchingSubPlans(PartitionPruneState *prunestate,
bool initial_prune);
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 2e132afc5a..b2d6f8fb6e 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1407,6 +1407,8 @@ typedef struct PlanRowMark
* Then, since an Append-type node could have multiple partitioning
* hierarchies among its children, we have an unordered List of those Lists.
*
+ * root_parent_relids RelOptInfo.relids of the relation to which the parent
+ * plan node and this PartitionPruneInfo node belong
* prune_infos List of Lists containing PartitionedRelPruneInfo nodes,
* one sublist per run-time-prunable partition hierarchy
* appearing in the parent plan node's subplans.
@@ -1419,6 +1421,7 @@ typedef struct PartitionPruneInfo
pg_node_attr(no_equal)
NodeTag type;
+ Bitmapset *root_parent_relids;
List *prune_infos;
Bitmapset *other_subplans;
} PartitionPruneInfo;
view thread (108+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <CA+HiwqG=yLd843jneu09fG+hFTQWYYw1xtxchh0hA5N+gofJhg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox