Re: Run-time pruning for ModifyTable
On Tue, Apr 7, 2020 at 8:52 PM David Rowley wrote: > On Wed, 25 Mar 2020 at 15:37, Amit Langote wrote: > > Actually, I was saying in that email that the update/delete planning > > overhaul being talked about will make the entirety of the > > functionality this patch is adding, which is ModifyTable node being > > able to prune its subplans based on run-time parameter values, > > redundant. That's because, with the overhaul, there won't be multiple > > subplans under ModifyTable, only one which would take care of any > > pruning that's necessary. > > Thanks for explaining. I've not read over any patch for that yet, so > wasn't aware of exactly what was planned. > > With your explanation, I imagine some sort of Append / MergeAppend > that runs the query as if it were a SELECT, but each > Append/MergeAppend subnode is tagged somehow with an index of which > ModifyTable subnode that it belongs to. Basically, just one complete > plan, rather than a plan per ModifyTable subnode. That's correct, although I don't think Append/MergeAppend will need to look any different structurally, except its subnodes will need to produce a targetlist member to identify partition/child for a given output row. There will still be N result relations, but not the N plans created separately for each, as inheritance_planner() currently does. > > What I did say in favor of this patch though is that it doesn not seem > > that invasive, so maybe okay to get in for v13. > > Since it seems there's much less code that will be useful after the > rewrite than I thought, combined with the fact that I'm not entirely > sure the best way to reuse the partitioned table's RelOptInfo from the > SELECT's PlannerInfo, then I'm going to return this one with feedback. That makes sense. I am thinking to spend some time working on this early in PG 14 cycle. -- Thank you, Amit Langote EnterpriseDB: http://www.enterprisedb.com
Re: Run-time pruning for ModifyTable
On Wed, 25 Mar 2020 at 15:37, Amit Langote wrote: > Actually, I was saying in that email that the update/delete planning > overhaul being talked about will make the entirety of the > functionality this patch is adding, which is ModifyTable node being > able to prune its subplans based on run-time parameter values, > redundant. That's because, with the overhaul, there won't be multiple > subplans under ModifyTable, only one which would take care of any > pruning that's necessary. Thanks for explaining. I've not read over any patch for that yet, so wasn't aware of exactly what was planned. With your explanation, I imagine some sort of Append / MergeAppend that runs the query as if it were a SELECT, but each Append/MergeAppend subnode is tagged somehow with an index of which ModifyTable subnode that it belongs to. Basically, just one complete plan, rather than a plan per ModifyTable subnode. > What I did say in favor of this patch though is that it doesn not seem > that invasive, so maybe okay to get in for v13. Since it seems there's much less code that will be useful after the rewrite than I thought, combined with the fact that I'm not entirely sure the best way to reuse the partitioned table's RelOptInfo from the SELECT's PlannerInfo, then I'm going to return this one with feedback. David
Re: Run-time pruning for ModifyTable
Hi David, Sorry I couldn't get to this sooner. On Wed, Mar 25, 2020 at 9:49 AM David Rowley wrote: > On Wed, 25 Mar 2020 at 13:00, Tom Lane wrote: > > David Rowley writes: > > > I had a closer look at this today and the code I have in > > > inheritance_planner() is certainly not right. > > > > Although I didn't get around to it for v13, there's still a plan on the > > table for inheritance_planner() to get nuked from orbit [1]. > > > > Maybe this improvement should be put on hold till that's done? > > Possibly. I'm not really wedded to the idea of getting it in. However, > it would really only be the inheritance planner part that would need > to be changed later. I don't think any of the other code would need to > be adjusted. > > Amit shared his thoughts in [1]. If you'd rather I held off, then I will. > > David > > [1] > https://www.postgresql.org/message-id/CA%2BHiwqGhD7ieKsv5%2BGkmHgs-XhP2DoUhtESVb3MU-4j14%3DG6LA%40mail.gmail.com Actually, I was saying in that email that the update/delete planning overhaul being talked about will make the entirety of the functionality this patch is adding, which is ModifyTable node being able to prune its subplans based on run-time parameter values, redundant. That's because, with the overhaul, there won't be multiple subplans under ModifyTable, only one which would take care of any pruning that's necessary. What I did say in favor of this patch though is that it doesn not seem that invasive, so maybe okay to get in for v13. -- Thank you, Amit Langote EnterpriseDB: http://www.enterprisedb.com
Re: Run-time pruning for ModifyTable
On Wed, 25 Mar 2020 at 13:00, Tom Lane wrote: > > David Rowley writes: > > I had a closer look at this today and the code I have in > > inheritance_planner() is certainly not right. > > Although I didn't get around to it for v13, there's still a plan on the > table for inheritance_planner() to get nuked from orbit [1]. > > Maybe this improvement should be put on hold till that's done? Possibly. I'm not really wedded to the idea of getting it in. However, it would really only be the inheritance planner part that would need to be changed later. I don't think any of the other code would need to be adjusted. Amit shared his thoughts in [1]. If you'd rather I held off, then I will. David [1] https://www.postgresql.org/message-id/CA%2BHiwqGhD7ieKsv5%2BGkmHgs-XhP2DoUhtESVb3MU-4j14%3DG6LA%40mail.gmail.com
Re: Run-time pruning for ModifyTable
David Rowley writes: > I had a closer look at this today and the code I have in > inheritance_planner() is certainly not right. Although I didn't get around to it for v13, there's still a plan on the table for inheritance_planner() to get nuked from orbit [1]. Maybe this improvement should be put on hold till that's done? regards, tom lane [1] https://www.postgresql.org/message-id/357.1550612935%40sss.pgh.pa.us
Re: Run-time pruning for ModifyTable
On Tue, 10 Mar 2020 at 00:13, David Rowley wrote: > Over in inheritance_planner(), I noticed that the RT index of the > SELECT query and the UPDATE/DELETE query can differ. There was some > code that performed translations. I changed that code slightly so that > it's a bit more optimal. It was building two lists, one for the old > RT index and one for the new. It added elements to this list > regardless of if the RT indexes were the same or not. I've now changed > that to only add to the list if they differ, which I feel should never > be slower and most likely always faster. I'm also now building a > translation map between the old and new RT indexes, however, I only > found one test in the regression tests which require any sort of > translation of these RT indexes. This was with an inheritance table, > so I need to do a bit more work to find a case where this happens with > a partitioned table to ensure all this works. I had a closer look at this today and the code I have in inheritance_planner() is certainly not right. It's pretty easy to made the SELECT and UPDATE/DELETE's RT indexes differ with something like: drop table part_t cascade; create table part_t (a int, b int, c int) partition by list (a); create table part_t12 partition of part_t for values in(1,2) partition by list (a); create table part_t12_1 partition of part_t12 for values in(1); create table part_t12_2 partition of part_t12 for values in(2); create table part_t3 partition of part_t for values in(3); create view vw_part_t as select * from part_t; explain analyze update vw_part_t set a = t2.a +0 from part_t t2 where t2.a = vw_part_t.a and vw_part_t.a = (select 1); In this case, the sub-partitioned table changes RT index. I can't just take the RelOptInfo's from the partition_root's simple_rel_array and put them in the correct element in the root's simple_rel_array as they RT indexes stored within also need to be translated. I'll be having another look at this to see what the best fix is going to be. David
Re: Run-time pruning for ModifyTable
Thanks for having a look at this, Amit. On Fri, 24 Jan 2020 at 21:57, Amit Langote wrote: > > On Thu, Jan 23, 2020 at 4:31 PM Amit Langote wrote: > Part of the optimizer patch that looks a bit complex is the changes to > inheritance_planner() which is to be expected, because that function > is a complex beast itself. I have suggestions to modify some comments > around the code added/modified by the patch for clarity; attaching a > delta patch for that. I've made another pass over the patch and made various changes. The biggest of which was the required modifications to nodeModifyTable.c so that it can now prune all partitions. Append and MergeAppend were modified to allow this in 5935917ce59 (Thanks for pushing that Tom). I've also slightly simplified the code in ExecModifyTable() and added slightly more code to ExecInitModifyTable(). We now only set mt_whichplan to WHICHPLAN_CHOOSE_SUBPLAN when run-time pruning is enabled and do_exec_prune is true. I also made it so when all partitions are pruned that we set mt_whichplan to WHICHPLAN_CHOOSE_SUBPLAN as this saves an additional run-time check during execution. Over in inheritance_planner(), I noticed that the RT index of the SELECT query and the UPDATE/DELETE query can differ. There was some code that performed translations. I changed that code slightly so that it's a bit more optimal. It was building two lists, one for the old RT index and one for the new. It added elements to this list regardless of if the RT indexes were the same or not. I've now changed that to only add to the list if they differ, which I feel should never be slower and most likely always faster. I'm also now building a translation map between the old and new RT indexes, however, I only found one test in the regression tests which require any sort of translation of these RT indexes. This was with an inheritance table, so I need to do a bit more work to find a case where this happens with a partitioned table to ensure all this works. > The executor patch looks pretty benign too. Diffs that looked a bit > suspicious at first are due to replacing > ModifyTableState.resultRelInfo that is a pointer into > EState.es_result_relations array by an array of ResultRelInfo > pointers, but doing that seems to make the relevant code easier to > follow, especially if you consider the changes that the patch makes to > that code. Yeah, that's because the ModifyTableState's resultRelInfo field was just a pointer to the estate->es_result_relations array offset by the ModifyTable's resultRelIndex. This was fine previously because we always initialised the plans for each ResultRelInfo. However, now that we might be pruning some of those that array can't be used as it'll still contain ResultRelInfos for relations we're not going to touch. Changing this to an array of pointers allows us to point to the elements in estate->es_result_relations that we're going to use. I also renamed the field just to ensure nothing can compile (thinking of extensions here) that's not got updated code. Tom, I'm wondering if you wouldn't mind looking over my changes to inheritance_planner()? David modifytable_runtime_prune_2020-03-10.patch Description: Binary data
Re: Run-time pruning for ModifyTable
On Thu, Jan 23, 2020 at 4:31 PM Amit Langote wrote:
> Now, the chances of such a big overhaul of how UPDATEs of inheritance
> trees are handled getting into PG 13 seem pretty thin even if I post
> the patch in few days, so perhaps it would make sense to get this
> patch in so that we can give users run-time pruning for UPDATE/DELETE
> in PG 13, provided the code is not very invasive. If and when the
> aforesaid overhaul takes place, that code would go away along with a
> lot of other code.
Fwiw, I updated the patch, mainly expected/partition_prune.out. Some
tests in it were failing as a fallout of commits d52eaa09 (pointed out
by Thomas upthread) and 6ef77cf46e8, which are not really related to
the code being changed by the patch.
On the patch itself, it seems straightforward enough. It simply takes
the feature we have for Append and MergeAppend nodes and adopts it for
ModifyTable which for the purposes of run-time pruning looks very much
like the aforementioned nodes.
Part of the optimizer patch that looks a bit complex is the changes to
inheritance_planner() which is to be expected, because that function
is a complex beast itself. I have suggestions to modify some comments
around the code added/modified by the patch for clarity; attaching a
delta patch for that.
The executor patch looks pretty benign too. Diffs that looked a bit
suspicious at first are due to replacing
ModifyTableState.resultRelInfo that is a pointer into
EState.es_result_relations array by an array of ResultRelInfo
pointers, but doing that seems to make the relevant code easier to
follow, especially if you consider the changes that the patch makes to
that code.
I'll set the CF entry to Needs Review, because AFAICS there are no
unaddressed comments.
Thanks,
Amit
diff --git a/src/backend/optimizer/plan/planner.c
b/src/backend/optimizer/plan/planner.c
index 30d15291e3..c4244e6d29 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1802,7 +1802,9 @@ inheritance_planner(PlannerInfo *root)
* When performing UPDATE/DELETE on a partitioned table, if the query
has
* a WHERE clause which supports it, we may be able to perform run-time
* partition pruning. The following code sets things up to allow this
to
-* be possible.
+* be possible using the information from partition_root that was used
+* during planning of the SELECT version of this query which we
performed
+* above.
*/
if (partition_root && !dummy_update)
{
@@ -1810,36 +1812,29 @@ inheritance_planner(PlannerInfo *root)
int i;
/*
-* Fetch the target partitioned table from the SELECT version of
-* the query which we performed above. This may have the base
quals
+* Fetch the target partitioned table which may have the base
quals
* which could allow the run-time pruning to work.
*/
parent_rel =
partition_root->simple_rel_array[top_parentRTindex];
-
final_rel->baserestrictinfo = parent_rel->baserestrictinfo;
- /* build a list of partitioned rels */
+ /* Collect all non-leaf tables in the partition tree being
updated. */
i = -1;
while ((i = bms_next_member(parent_relids, i)) > 0)
partitioned_rels = lappend_int(partitioned_rels, i);
-
/*
-* In order to build the run-time pruning data we'll need
append rels
-* any sub-partitioned tables. If there are some of those and
the
-* append_rel_array is not already allocated, then do that now.
+* There can only be a single partition tree, the one whose
root is
+* the query's main target table.
*/
- if (list_length(partitioned_rels) > 1 &&
- root->append_rel_array == NULL)
- root->append_rel_array = palloc0(sizeof(AppendRelInfo
*) *
-
root->simple_rel_array_size);
+ partitioned_rels = list_make1(partitioned_rels);
/*
-* There can only be a single partition hierarchy, so it's fine
to
-* just make a single element list of the partitioned_rels.
+* Update simple_rel_array and append_rel_array so that runtime
+* pruning setup logic can find the relavant partitioned
relations.
+* Just use the one that the planning of SELECT version of the
query
+* would have created.
*/
- partitioned_rels = list_make1(partitioned_rels);
-
i = -1;
while ((i = bms_next_member(parent_relids, i)) >= 0)
{
@@ -1847,11
Re: Run-time pruning for ModifyTable
Sorry, I didn't notice this email until now. On Wed, Nov 27, 2019 at 5:17 PM Michael Paquier wrote: > On Tue, Nov 05, 2019 at 04:04:25PM +1300, Thomas Munro wrote: > > On Thu, Sep 12, 2019 at 10:10 AM Alvaro Herrera > > wrote: > > > Here's a rebased version of this patch (it had a trivial conflict). > > > > Hi, FYI partition_prune.sql currently fails (maybe something to do > > with commit d52eaa09?): > > David, perhaps you did not notice that? For now I have moved this > patch to next CF waiting on author to look after the failure. > > Amit, Kato-san, both of you are marked as reviewers of this patch. > Are you planning to look at it? Sorry, I never managed to look at the patch closely. As I commented up-thread, the functionality added by this patch would be unnecessary if we were to move forward with the other project related to UPDATE and DELETE over inheritance trees: https://www.postgresql.org/message-id/357.1550612935%40sss.pgh.pa.us I had volunteered to submit a patch in that thread and even managed to write one but didn't get time to get it in good enough shape to post it to the list, like I couldn't make it handle foreign child tables. The gist of the new approach is that ModifyTable will always have *one* subplan under ModifyTable, not N for N target partitions as currently. That one subplan being the same plan as one would get if the query were SELECT instead of UPDATE/DELETE, it would automatically take care of run-time pruning if needed, freeing ModifyTable itself from having to do it. Now, the chances of such a big overhaul of how UPDATEs of inheritance trees are handled getting into PG 13 seem pretty thin even if I post the patch in few days, so perhaps it would make sense to get this patch in so that we can give users run-time pruning for UPDATE/DELETE in PG 13, provided the code is not very invasive. If and when the aforesaid overhaul takes place, that code would go away along with a lot of other code. Thanks, Amit
Re: Run-time pruning for ModifyTable
On Thu, Jan 16, 2020 at 10:45:25PM +0100, Tomas Vondra wrote: > David, this patch is marked as "waiting on author" since 11/27, and > there have been no updates or responses since then. Do you plan to > submit a new patch version in this CF? We're already half-way through, > so there's not much time ... The reason why I moved it to 2020-01 is that there was not enough time for David to reply back. At this stage, it seems more appropriate to me to mark it as returned with feedback and move on. -- Michael signature.asc Description: PGP signature
Re: Run-time pruning for ModifyTable
On Wed, Nov 27, 2019 at 05:17:06PM +0900, Michael Paquier wrote: On Tue, Nov 05, 2019 at 04:04:25PM +1300, Thomas Munro wrote: On Thu, Sep 12, 2019 at 10:10 AM Alvaro Herrera wrote: > Here's a rebased version of this patch (it had a trivial conflict). Hi, FYI partition_prune.sql currently fails (maybe something to do with commit d52eaa09?): David, perhaps you did not notice that? For now I have moved this patch to next CF waiting on author to look after the failure. Amit, Kato-san, both of you are marked as reviewers of this patch. Are you planning to look at it? David, this patch is marked as "waiting on author" since 11/27, and there have been no updates or responses since then. Do you plan to submit a new patch version in this CF? We're already half-way through, so there's not much time ... regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Run-time pruning for ModifyTable
On Tue, Nov 05, 2019 at 04:04:25PM +1300, Thomas Munro wrote: > On Thu, Sep 12, 2019 at 10:10 AM Alvaro Herrera > wrote: > > Here's a rebased version of this patch (it had a trivial conflict). > > Hi, FYI partition_prune.sql currently fails (maybe something to do > with commit d52eaa09?): David, perhaps you did not notice that? For now I have moved this patch to next CF waiting on author to look after the failure. Amit, Kato-san, both of you are marked as reviewers of this patch. Are you planning to look at it? -- Michael signature.asc Description: PGP signature
Re: Run-time pruning for ModifyTable
On Thu, Sep 12, 2019 at 10:10 AM Alvaro Herrera wrote: > Here's a rebased version of this patch (it had a trivial conflict). Hi, FYI partition_prune.sql currently fails (maybe something to do with commit d52eaa09?): where s.a = $1 and s.b = $2 and s.c = (select 1); explain (costs off) execute q (1, 1); QUERY PLAN + Append InitPlan 1 (returns $0) -> Result - Subplans Removed: 1 -> Seq Scan on p1 - Filter: ((a = $1) AND (b = $2) AND (c = $0)) + Filter: ((a = 1) AND (b = 1) AND (c = $0)) -> Seq Scan on q111 - Filter: ((a = $1) AND (b = $2) AND (c = $0)) + Filter: ((a = 1) AND (b = 1) AND (c = $0)) -> Result - One-Time Filter: ((1 = $1) AND (1 = $2) AND (1 = $0)) -(10 rows) + One-Time Filter: (1 = $0) +(9 rows) execute q (1, 1); a | b | c
Re: Run-time pruning for ModifyTable
Here's a rebased version of this patch (it had a trivial conflict).
No further changes.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 82d8140ba2..677b9cdd58 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1951,7 +1951,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate,
if (plan && plan->operation == CMD_UPDATE &&
(resultRelInfo->ri_usesFdwDirectModify ||
resultRelInfo->ri_FdwState) &&
- resultRelInfo > mtstate->resultRelInfo + mtstate->mt_whichplan)
+ resultRelInfo > mtstate->resultRelInfos[mtstate->mt_whichplan])
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot route tuples into foreign table to be updated \"%s\"",
@@ -2005,7 +2005,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate,
*/
if (plan && plan->operation == CMD_UPDATE &&
resultRelation == plan->rootRelation)
- resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+ resultRelation = mtstate->resultRelInfos[0]->ri_RangeTableIndex;
}
/* Construct the SQL command string. */
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 3aeef30b28..2e9954ddda 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2856,7 +2856,7 @@ CopyFrom(CopyState cstate)
mtstate->ps.plan = NULL;
mtstate->ps.state = estate;
mtstate->operation = CMD_INSERT;
- mtstate->resultRelInfo = estate->es_result_relations;
+ mtstate->resultRelInfos = >es_result_relations;
if (resultRelInfo->ri_FdwRoutine != NULL &&
resultRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 62fb3434a3..682bf615db 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3190,14 +3190,14 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
/* Should we explicitly label target relations? */
labeltargets = (mtstate->mt_nplans > 1 ||
(mtstate->mt_nplans == 1 &&
- mtstate->resultRelInfo->ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfos[0]->ri_RangeTableIndex != node->nominalRelation));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
for (j = 0; j < mtstate->mt_nplans; j++)
{
- ResultRelInfo *resultRelInfo = mtstate->resultRelInfo + j;
+ ResultRelInfo *resultRelInfo = mtstate->resultRelInfos[j];
FdwRoutine *fdwroutine = resultRelInfo->ri_FdwRoutine;
if (labeltargets)
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index d23f292cb0..8d1d961809 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -471,7 +471,7 @@ ExecHashSubPlanResultRelsByOid(ModifyTableState *mtstate,
/* Hash all subplans by their Oid */
for (i = 0; i < mtstate->mt_nplans; i++)
{
- ResultRelInfo *rri = >resultRelInfo[i];
+ ResultRelInfo *rri = mtstate->resultRelInfos[i];
bool found;
Oid partoid = RelationGetRelid(rri->ri_RelationDesc);
SubplanResultRelHashElem *elem;
@@ -508,7 +508,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
Relation rootrel = rootResultRelInfo->ri_RelationDesc,
partrel;
- Relation firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
+ Relation firstResultRel = mtstate->resultRelInfos[0]->ri_RelationDesc;
ResultRelInfo *leaf_part_rri;
MemoryContext oldcxt;
AttrNumber *part_attnos = NULL;
@@ -556,7 +556,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
List *wcoList;
List *wcoExprs = NIL;
ListCell *ll;
- int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+ int firstVarno = mtstate->resultRelInfos[0]->ri_RangeTableIndex;
/*
* In the case of INSERT on a partitioned table, there is only one
@@ -621,7 +621,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
TupleTableSlot *slot;
ExprContext *econtext;
List *returningList;
- int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+ int firstVarno = mtstate->resultRelInfos[0]->ri_RangeTableIndex;
/* See the comment above for WCO lists. */
Assert((node->operation == CMD_INSERT &&
@@ -681,7 +681,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
*/
if (node && node->onConflictAction != ONCONFLICT_NONE)
{
- int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+ int firstVarno = mtstate->resultRelInfos[0]->ri_RangeTableIndex;
TupleDesc partrelDesc = RelationGetDescr(partrel);
ExprContext *econtext = mtstate->ps.ps_ExprContext;
ListCell *lc;
diff --git a/src/backend/executor/nodeModifyTable.c
RE: Run-time pruning for ModifyTable
On Monday, July 8, 2019 11:34 AM, Amit Langote wrote: > By the way, let's keep any further discussion on this particular topic > in the other thread. Thanks for the details. I got it. Regards, Kato sho > -Original Message- > From: Amit Langote [mailto:amitlangot...@gmail.com] > Sent: Monday, July 8, 2019 11:34 AM > To: Kato, Sho/加藤 翔 > Cc: David Rowley ; PostgreSQL Hackers > > Subject: Re: Run-time pruning for ModifyTable > > Kato-san, > > On Thu, Jul 4, 2019 at 1:40 PM Kato, Sho wrote: > > > If I understand the details of [1] correctly, ModifyTable will no > > > longer have N subplans for N result relations as there are today. > > > So, it doesn't make sense for ModifyTable to contain > > > PartitionedRelPruneInfos and for > ExecInitModifyTable/ExecModifyTable > > > to have to perform initial and execution-time pruning, respectively. > > > > Does this mean that the generic plan will not have N subplans for N > result relations? > > I thought [1] would make creating generic plans faster, but is this > correct? > > Yeah, making a generic plan for UPDATE of inheritance tables will > certainly become faster, because we will no longer plan the same query > N times for N child tables. There will still be N result relations but > only one sub-plan to fetch the rows from. Also, planning will still cost > O(N), but with a much smaller constant factor. > > By the way, let's keep any further discussion on this particular topic > in the other thread. > > Thanks, > Amit
Re: Run-time pruning for ModifyTable
Kato-san, On Thu, Jul 4, 2019 at 1:40 PM Kato, Sho wrote: > > If I understand the details of [1] correctly, ModifyTable will no longer > > have N subplans for N result relations as there are today. So, it doesn't > > make sense for ModifyTable to contain PartitionedRelPruneInfos and for > > ExecInitModifyTable/ExecModifyTable > > to have to perform initial and execution-time pruning, respectively. > > Does this mean that the generic plan will not have N subplans for N result > relations? > I thought [1] would make creating generic plans faster, but is this correct? Yeah, making a generic plan for UPDATE of inheritance tables will certainly become faster, because we will no longer plan the same query N times for N child tables. There will still be N result relations but only one sub-plan to fetch the rows from. Also, planning will still cost O(N), but with a much smaller constant factor. By the way, let's keep any further discussion on this particular topic in the other thread. Thanks, Amit
RE: Run-time pruning for ModifyTable
Hi, Amit > If I understand the details of [1] correctly, ModifyTable will no longer > have N subplans for N result relations as there are today. So, it doesn't > make sense for ModifyTable to contain PartitionedRelPruneInfos and for > ExecInitModifyTable/ExecModifyTable > to have to perform initial and execution-time pruning, respectively. Does this mean that the generic plan will not have N subplans for N result relations? I thought [1] would make creating generic plans faster, but is this correct? regards, kato sho > -Original Message- > From: Amit Langote [mailto:amitlangot...@gmail.com] > Sent: Wednesday, July 3, 2019 5:41 PM > To: David Rowley > Cc: PostgreSQL Hackers > Subject: Re: Run-time pruning for ModifyTable > > On Wed, Jul 3, 2019 at 4:34 PM David Rowley > wrote: > > On Wed, 3 Jul 2019 at 17:27, Amit Langote > wrote: > > > A cursory look at the patch suggests that most of its changes will > > > be for nothing if [1] materializes. What do you think about that? > > > > Yeah, I had this in mind when writing the patch, but kept going > > anyway. I think it's only really a small patch of this patch that > > would get wiped out with that change. Just the planner.c stuff. > > Everything else is still required, as far as I understand. > > If I understand the details of [1] correctly, ModifyTable will no longer > have N subplans for N result relations as there are today. So, it doesn't > make sense for ModifyTable to contain PartitionedRelPruneInfos and for > ExecInitModifyTable/ExecModifyTable > to have to perform initial and execution-time pruning, respectively. > As I said, bottom expansion of target inheritance will mean pruning (both > plan-time and run-time) will occur at the bottom too, so the run-time > pruning capabilities of nodes that already have it will be used for UPDATE > and DELETE too. > > Thanks, > Amit >
Re: Run-time pruning for ModifyTable
On Wed, Jul 3, 2019 at 4:34 PM David Rowley wrote: > On Wed, 3 Jul 2019 at 17:27, Amit Langote wrote: > > A cursory look at the patch suggests that most of its changes will be > > for nothing if [1] materializes. What do you think about that? > > Yeah, I had this in mind when writing the patch, but kept going > anyway. I think it's only really a small patch of this patch that > would get wiped out with that change. Just the planner.c stuff. > Everything else is still required, as far as I understand. If I understand the details of [1] correctly, ModifyTable will no longer have N subplans for N result relations as there are today. So, it doesn't make sense for ModifyTable to contain PartitionedRelPruneInfos and for ExecInitModifyTable/ExecModifyTable to have to perform initial and execution-time pruning, respectively. As I said, bottom expansion of target inheritance will mean pruning (both plan-time and run-time) will occur at the bottom too, so the run-time pruning capabilities of nodes that already have it will be used for UPDATE and DELETE too. Thanks, Amit
Re: Run-time pruning for ModifyTable
On Wed, 3 Jul 2019 at 17:27, Amit Langote wrote: > A cursory look at the patch suggests that most of its changes will be > for nothing if [1] materializes. What do you think about that? Yeah, I had this in mind when writing the patch, but kept going anyway. I think it's only really a small patch of this patch that would get wiped out with that change. Just the planner.c stuff. Everything else is still required, as far as I understand. > [1] https://www.postgresql.org/message-id/357.1550612935%40sss.pgh.pa.us -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Run-time pruning for ModifyTable
Hi David, On Thu, Jun 27, 2019 at 2:28 PM David Rowley wrote: > Deja vu from this time last year when despite everyone's best efforts > (mostly Alvaro) we missed getting run-time pruning in for MergeAppend > into the PG11 release. This year it was ModifyTable, which is now > possible thanks to Amit and Tom's modifications to the inheritance > planner. > > I've attached what I have so far for this. Thanks for working on this. IIUC, the feature is to skip modifying a given result relation if run-time pruning dictates that none of its existing rows will match some dynamically computable quals. > I think it's mostly okay, > but my brain was overheating a bit at the inheritance_planner changes. I think we need to consider the fact that there is a proposal [1] to get rid of inheritance_planner() as the way of planning UPDATE/DELETEs on inheritance trees. If we go that route, then a given partitioned target table will be expanded at the bottom and so, there's no need for ModifyTable to have its own run-time pruning info, because Append/MergeAppend will have it. Maybe, we will need some code in ExecInitModifyTable() and ExecModifyTable() to handle the case where run-time pruning, during plan tree initialization and plan tree execution respectively, may have rendered modifying a given result relation unnecessary. A cursory look at the patch suggests that most of its changes will be for nothing if [1] materializes. What do you think about that? Thanks, Amit [1] https://www.postgresql.org/message-id/357.1550612935%40sss.pgh.pa.us
