David Rowley <david.row...@2ndquadrant.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
