Re: Patch for removng unused targets

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Josh Berkus <josh(at)agliodbs(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch for removng unused targets
Date: 2013-08-02 22:45:56
Message-ID: 10575.1375483556@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> Reading between the lines of the original submission at
>> <CAPpHfdtG5qoHoD+w=Tz3wC3fZ=b8i21=V5xandBFM=DTo-Yg=Q(at)mail(dot)gmail(dot)com>,
>> I gather that it's the KNNGist-style case that worries you, so maybe
>> it's worth applying this type of patch anyway. I'd want to rejigger
>> it to be aware of the cost implications though, at least for
>> grouping_planner's choices.

> Hmm. Can we optimize for the KNN case, without causing the issues which
> you warned about earlier in your post?

Those are pre-existing issues, not something that would be made any worse
by this patch. The main thing I think is really wrong with the patch
as it stands is that the cost savings from suppressing the ORDER BY
expressions should enter into the cheapest_path-vs-sorted_path decision,
which it doesn't, in fact the total cost the plan is labeled with isn't
corrected either. (Not that that matters for the current level of plan,
but it could matter at an outer level if this is a subquery.) I think
that is fixable but am just wondering whether to bother.

> So, Returned With Feedback, or move it to September?

The patch is certainly not getting committed as-is (at least not by me),
so it would likely be fair to mark it RWF so we can close the commitfest.
I'll still work on a revised version after the fest if people think that
improving the KNN-search case is worth a patch that's a bit larger than
this one currently is.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2013-08-02 22:55:27 Re: Kudos for Reviewers -- wrapping it up
Previous Message Bruce Momjian 2013-08-02 22:18:10 Re: Kudos for Reviewers -- wrapping it up