Moving SS_finalize_plan processing to the end of planning
I've started to work on path-ification of the upper planner (finally),
and since that's going to be a large patch in any case, I've been looking
for pieces that could be bitten off and done separately. One such piece
is the fact that SS_finalize_plan (computation of extParams/allParams)
currently gets run at the end of subquery_planner; as long as that's true,
we cannot have subquery_planner returning paths rather than concrete
plans. The attached patch moves that processing to occur just before
set_plan_references is run.
Ideally I'd like to get rid of SS_finalize_plan altogether and merge its
work into set_plan_references, so as to save one traversal of the finished
plan tree. However, that's a bit difficult because of the fact that the
traversal order is different: in SS_finalize_plan, we must visit subplans
before main plan, whereas set_plan_references wants to do the main plan
first. (I experimented with changing that, but then the flat rangetable
comes out in a different order, with unpleasant side effects on the way
EXPLAIN prints things.) Since that would be purely a minor performance
improvement, I set that goal aside for now.
Basically what this patch does is to divide what had been done in
SS_finalize_plan into three steps:
* SS_identify_outer_params determines which outer-query-level Params will
be available for the current query level to use, and saves that aside in
a new PlannerInfo field root->outer_params. This processing turns out
to be the only reason that SS_finalize_plan had to be called in
subquery_planner: we have to capture this data before exiting
subquery_planner because the upper levels' plan_params lists may change
as they plan other subplans.
* SS_attach_initplans does the work of attaching initplans to the parent
plan node and adjusting the parent's cost estimate accordingly.
* SS_finalize_plan now *only* does extParam/allParam calculation.
I had to change things around a bit in planagg.c (which was already a
hack, and the law of conservation of cruft applies). Otherwise it's
pretty straightforward and removes some existing hacks. One notable
point is that there's no longer an assumption within SS_finalize_plan
that initPlans can only appear in the top plan node.
Any objections?
regards, tom lane
Attachments:
run-SS_finalize_plan-later-1.patchtext/x-diff; charset=us-ascii; name=run-SS_finalize_plan-later-1.patchDownload+412-266
On 10 August 2015 at 07:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I've started to work on path-ification of the upper planner (finally),
and since that's going to be a large patch in any case, I've been looking
for pieces that could be bitten off and done separately. One such piece
is the fact that SS_finalize_plan (computation of extParams/allParams)
currently gets run at the end of subquery_planner; as long as that's true,
we cannot have subquery_planner returning paths rather than concrete
plans. The attached patch moves that processing to occur just before
set_plan_references is run.
Basically what this patch does is to divide what had been done in
SS_finalize_plan into three steps:* SS_identify_outer_params determines which outer-query-level Params will
be available for the current query level to use, and saves that aside in
a new PlannerInfo field root->outer_params. This processing turns out
to be the only reason that SS_finalize_plan had to be called in
subquery_planner: we have to capture this data before exiting
subquery_planner because the upper levels' plan_params lists may change
as they plan other subplans.* SS_attach_initplans does the work of attaching initplans to the parent
plan node and adjusting the parent's cost estimate accordingly.* SS_finalize_plan now *only* does extParam/allParam calculation.
I had to change things around a bit in planagg.c (which was already a
hack, and the law of conservation of cruft applies). Otherwise it's
pretty straightforward and removes some existing hacks. One notable
point is that there's no longer an assumption within SS_finalize_plan
that initPlans can only appear in the top plan node.Any objections?
Great! I'm very interested in this work, specifically around the
grouping_planner() changes too.
I've looked over the patch and from what I understand it seems like a good
solid step in the right direction.
I was digging around the grouping_planner() last week with the intentions
of making some changes there to allow GROUP BY before join, but in the end
decided to stay well clear of this area until this pathification is done.
So far I've managed to keep my changes away from the upper planner and I've
added "GroupingPath" types, which from what I can predict of what you'll be
making changes to, I think you'll also need to have grouping_planner()
return a few variations of "GroupingPath" to allow the paths list to be
passed up to subquery_planner() and on up to functions
like recurse_set_operations() so that they have the option of choosing
GroupAggregate / MergeAppend to implement UNION.
If I'm right on this, then maybe there's a few things you can copy and
paste from the patch I posted here:
/messages/by-id/CAKJS1f-sEcm=gTfS-DqjsOcsZ-vLhrP_hSRNtJjq-S7Egn8Rqw@mail.gmail.com
specifically around create_grouping_path()?
Kind Regards
David Rowley
--
David Rowley http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Training & Services
David Rowley <david.rowley@2ndquadrant.com> writes:
On 10 August 2015 at 07:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I've started to work on path-ification of the upper planner (finally),
I was digging around the grouping_planner() last week with the intentions
of making some changes there to allow GROUP BY before join, but in the end
decided to stay well clear of this area until this pathification is done.
So far I've managed to keep my changes away from the upper planner and I've
added "GroupingPath" types, which from what I can predict of what you'll be
making changes to, I think you'll also need to have grouping_planner()
return a few variations of "GroupingPath" to allow the paths list to be
passed up to subquery_planner() and on up to functions
like recurse_set_operations() so that they have the option of choosing
GroupAggregate / MergeAppend to implement UNION.
If I'm right on this, then maybe there's a few things you can copy and
paste from the patch I posted here:
/messages/by-id/CAKJS1f-sEcm=gTfS-DqjsOcsZ-vLhrP_hSRNtJjq-S7Egn8Rqw@mail.gmail.com
specifically around create_grouping_path()?
Yeah, I saw your patch, but have not yet had time to think about what
parts of it I could borrow.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 9, 2015 at 3:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I've started to work on path-ification of the upper planner (finally),
I don't have any specific technical comments on what you've proposed
here, but I'm excited to hear that this is moving forward.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Aug 9, 2015 at 3:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I've started to work on path-ification of the upper planner (finally),
and since that's going to be a large patch in any case, I've been looking
for pieces that could be bitten off and done separately. One such piece
is the fact that SS_finalize_plan (computation of extParams/allParams)
currently gets run at the end of subquery_planner; as long as that's true,
we cannot have subquery_planner returning paths rather than concrete
plans. The attached patch moves that processing to occur just before
set_plan_references is run.
Tom,
Do you expect to do more work on the upper planner path-ification stuff soon?
Just checking.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
Do you expect to do more work on the upper planner path-ification stuff soon?
Yeah, I do actually have some work in progress. Not sure how soon I'll be
ready to show it --- there's a lot of code to break and reassemble :-(
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Sep 10, 2015 at 9:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Do you expect to do more work on the upper planner path-ification stuff soon?
Yeah, I do actually have some work in progress. Not sure how soon I'll be
ready to show it --- there's a lot of code to break and reassemble :-(
Yeah, I remember looking at it a bit and finding it quite an
intimidating piece of code. I'm just asking because it would be nice
if we got the infrastructure into this release in time to do something
with it - specifically, I'd like to find a way for FDWs to push down
aggregates, a capability for which EDB has had multiple customer
requests (and I'm sure we're not the only ones).
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Sep 10, 2015 at 9:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Do you expect to do more work on the upper planner path-ification stuff soon?
Yeah, I do actually have some work in progress. Not sure how soon I'll be
ready to show it --- there's a lot of code to break and reassemble :-(
Yeah, I remember looking at it a bit and finding it quite an
intimidating piece of code. I'm just asking because it would be nice
if we got the infrastructure into this release in time to do something
with it - specifically, I'd like to find a way for FDWs to push down
aggregates, a capability for which EDB has had multiple customer
requests (and I'm sure we're not the only ones).
Yeah, I'm well aware that this is blocking progress in several areas.
I do intend to make it happen, but it's not the only demand on my time.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Sep 11, 2015 at 9:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Sep 10, 2015 at 9:49 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Do you expect to do more work on the upper planner path-ification stuff soon?
Yeah, I do actually have some work in progress. Not sure how soon I'll be
ready to show it --- there's a lot of code to break and reassemble :-(Yeah, I remember looking at it a bit and finding it quite an
intimidating piece of code. I'm just asking because it would be nice
if we got the infrastructure into this release in time to do something
with it - specifically, I'd like to find a way for FDWs to push down
aggregates, a capability for which EDB has had multiple customer
requests (and I'm sure we're not the only ones).Yeah, I'm well aware that this is blocking progress in several areas.
I do intend to make it happen, but it's not the only demand on my time.
Sure, I understand that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers