Re: Patch for removng unused targets - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: Patch for removng unused targets
Date
Msg-id CAPpHfdsv8z89=PnqvRkGztDzrY0O_DOh2QqRumZ1ewwtW7-f9g@mail.gmail.com
Whole thread Raw
In response to Re: Patch for removng unused targets  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Patch for removng unused targets  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Mon, Dec 3, 2012 at 8:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> Sorry for the delay.  I've reviewed the patch.  It was applied
> successfully, and it worked well for tests I did including the example
> you showed.  I think it's worth the work, but I'm not sure you go
> about it in the right way.  (I feel the patch decreases code
> readability more than it gives an advantage.)

One thought here is that I don't particularly like adding a field like
"resorderbyonly" to TargetEntry in the first place.  That makes this
optimization the business of the parser, which it should not be; and
furthermore makes it incumbent on the rewriter, as well as anything else
that manipulates parsetrees, to maintain the flag correctly while
rearranging queries.  It would be better if this were strictly the
business of the planner.

But having said that, I'm wondering (without having read the patch)
why you need anything more than the existing "resjunk" field.

Actually, I don't know all the cases when "resjunk" flag is set. Is it reliable to decide target to be used only for "ORDER BY" if it's "resjunk" and neither system or used in grouping? If it's so or there are some other cases which are easy to determine then I'll remove "resorderbyonly" flag.
 
------
With best regards,
Alexander Korotkov.

pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: WIP: store additional info in GIN index
Next
From: Alexander Korotkov
Date:
Subject: Re: WIP: store additional info in GIN index