From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Removing [Merge]Append nodes which contain a single subpath |
Date: | 2019-03-25 20:00:11 |
Message-ID: | 22384.1553544011@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
> [ drop-useless-merge-appends-15.patch ]
Pushed with some minor adjustments.
> I can get a plan that does end up with Result nodes above a Scan node.
> create table rangep (a int, b int) partition by range (a);
> create table rangep1 partition of rangep for values from(0) to (1000000);
> explain select r1::text from rangep r1 inner join rangep r2 on
> r1::text = r2::Text order by r1::text;
> However, if we modify is_projection_capable_plan() similar to how
> ExecSupportsMarkRestore() has been modified then we'd need to ensure
> that any changes made to the Append/MergeAppend's tlist also are made
> to the underlying node's tlist. I'm unsure if that's worth the
> complexity. What do you think?
I'm inclined to think not, at least not without more compelling
examples than that one ;-). Note that we had Result-above-the-Append
before, so we're not regressing for such cases, we're just leaving
something on the table.
>> One other remark is that the division of labor between
>> create_[merge]append_path and their costsize.c subroutines
>> seems pretty unprincipled. I'd be inclined to push all the
>> relevant logic into costsize.c, but have not done so here.
>> Moving the existing cost calculations in create_mergeappend_path
>> into costsize.c would better be done as a separate refactoring patch,
>> perhaps.
> I've not touched that, but happy to do it as a separate patch.
After looking at this, I'm less excited than I was before.
It seems it'd be only marginally less crufty than the way it is now.
In particular, costsize.c would have to be aware of the rule about
single-child MergeAppend being removable, which seems a little outside
its purview: typically we only ask costsize.c to estimate the cost
of a plan node as-presented, not to make decisions about what the
shape of the plan tree will be. So I left it alone.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Julien Rouhaud | 2019-03-25 20:03:45 | Re: Ordered Partitioned Table Scans |
Previous Message | Andrew Dunstan | 2019-03-25 19:44:45 | Re: MSVC Build support with visual studio 2019 |