Re: Patch for removng unused targets

From: "Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "'Alexander Korotkov'" <aekorotkov(at)gmail(dot)com>
Cc: "'pgsql-hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch for removng unused targets
Date: 2013-04-08 09:55:27
Message-ID: 001201ce343f$32cc5900$98650b00$@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]

> Alexander Korotkov <aekorotkov(at)gmail(dot)com> writes:
> > On Mon, Dec 3, 2012 at 8:31 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> 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.
>
> 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. Please find attached the patch.

> A more invasive, but possibly cleaner in the long run, approach is to
> strip all resjunk targets from the query's tlist at the start of
> planning and only put them back if needed.
>
> BTW, when I looked at this a couple years ago, it seemed like the major
> problem was that the planner assumes that all plans for the query should
> emit the same tlist, and thus that tlist eval cost isn't a
> distinguishing factor. Breaking that assumption seemed to require
> rather significant refactoring. I never found the time to try to
> actually do it.

Such an approach would improve code readability, but I'm not sure it's worth the
work for this optimization, though I think I'm missing something.

Thanks,

Best regards,
Etsuro Fujita

Attachment Content-Type Size
unused-targets-2.patch application/octet-stream 4.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Samrat Revagade 2013-04-08 10:34:21 Inconsistent DB data in Streaming Replication
Previous Message Simon Riggs 2013-04-08 08:19:00 Re: corrupt pages detected by enabling checksums