Re: Patch for removng unused targets - Mailing list pgsql-hackers
| From | Etsuro Fujita |
|---|---|
| Subject | Re: Patch for removng unused targets |
| Date | |
| Msg-id | 00d301ce6ce3$1f08ced0$5d1a6c70$@lab.ntt.co.jp Whole thread Raw |
| In response to | Re: Patch for removng unused targets (Hitoshi Harada <umi.tanuki@gmail.com>) |
| Responses |
Re: Patch for removng unused targets
|
| List | pgsql-hackers |
Hi Harada-san,
Thank you for the review.
I think that the parse tree has enough information to do this optimization and that the easiest way to do it is to use the information, though I might not have understand your comments correctly. So, I would like to fix the bug by simply modifying the removability check in adjust_targetlist() so that the resjunk column is not used in GROUP BY, DISTINCT ON and *window PARTITION/ORDER BY*, besides ORDER BY. No? I am open to any comments.
Thanks,
Best regards,
Etsuro Fujita
From: Hitoshi Harada [mailto:umi.tanuki@gmail.com]
Sent: Wednesday, June 19, 2013 2:57 PM
To: Etsuro Fujita
Cc: Tom Lane; Alexander Korotkov; pgsql-hackers
Subject: Re: [HACKERS] Patch for removng unused targets
On Tue, Jun 18, 2013 at 5:15 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
Hi Alexander,
I wrote:
> > > From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
>
> > > resjunk means that the target is not supposed to be output by the query.
> > > Since it's there at all, it's presumably referenced by ORDER BY or GROUP
> > > BY or DISTINCT ON, but the meaning of the flag doesn't depend on that.
>
> > > What you would need to do is verify that the target is resjunk and not
> > > used in any clause besides ORDER BY. I have not read your patch, but
> > > I rather imagine that what you've got now is that the parser checks this
> > > and sets the new flag for consumption far downstream. Why not just make
> > > the same check in the planner?
>
> > I've created a patch using this approach.
>
> I've rebased the above patch against the latest head. Could you review the
> patch? If you have no objection, I'd like to mark the patch "ready for
> committer".
Sorry, I've had a cleanup of the patch. Please find attached the patch.
Don't forget about window functions!
test=# EXPLAIN (ANALYZE, VERBOSE) SELECT *, count(*) over (partition by slow_func(x,y)) FROM test ORDER BY slow_func(x,y) LIMIT 10; QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------- Limit (cost=0.28..3.52 rows=10 width=16) (actual time=20.860..113.764 rows=10 loops=1)
Output: x, y, (count(*) OVER (?))
-> WindowAgg (cost=0.28..324.27 rows=1000 width=16) (actual time=20.858..113.747 rows=10 loops=1)
Output: x, y, count(*) OVER (?)
-> Index Scan using test_idx on public.test (cost=0.28..59.27 rows=1000 width=16) (actual time=10.563..113.530 rows=11 loops=1)
Output: slow_func(x, y), x, y
Total runtime: 117.889 ms
(7 rows)
And I don't think it's a good idea to rely on the parse tree to see if we can remove those unused columns from the target list, because there should be a lot of optimization that has been done through grouping_planner, and the parse tree is not necessarily representing the corresponding elements at this point. I think it'd be better to see path keys to find out the list of elements that may be removed, rather than SortClause, which would be a more generalized approach.
Thanks,
--
Hitoshi Harada
pgsql-hackers by date: