Re: Identity projection

On Friday, December 14, 2012 5:11 PM Heikki Linnakangas wrote:
> On 12.11.2012 12:07, Kyotaro HORIGUCHI wrote:
> > Hello, This is new version of identity projection patch.
> >
> > Reverted projectionInfo and ExecBuildProjectionInfo. Identity
> > projection is recognized directly in ExecGroup, ExecResult, and
> > ExecWindowAgg. nodeAgg is reverted because I couldn't make it
> > sane..
> >
> > The following is the result of performance test posted before in
> > order to show the source of the gain.
>
> Hmm, this reminds me of the discussion on removing useless Limit nodes:
> http://archives.postgresql.org/pgsql-performance/2012-12/msg00127.php.
>
> The optimization on Group, WindowAgg and Agg nodes doesn't seem that
> important, the cost of doing the aggregation/grouping is likely
> overwhelming the projection cost, and usually you do projection in
> grouping/aggregation anyway. But makes sense for Result.
>
> For Result, I think you should aim to remove the useless Result node
> from the plan altogether.

I was reviewing this patch and found that it can be move forward as follows:

There can be 2 ways to remove result node
a. Remove the Result plan node in case it is not required - This is same as
currently it does for SubqueryScan.
We can check if the result plan is trivial (with logic similar to
trivial_subqueryscan()), then remove result node.

b. to avoid adding it to Plan node in case it is not required -
For this, in grouping_planner() currently it checks if the plan is
projection capable (is_projection_capable_plan()),
we can enhance this check such that it also check projection is really
required depending if the targetlist contains
any non Var element.

Please suggest which way is more preferable and if one of above 2 seems to
be okay,
I can update the patch on behalf of Kyotaro-san incase they don't have time
currently.

> And do the same for useless Limit nodes.
Is it okay, if this can be done as part of separate patch?

With Regards,
Amit Kapila.