Re: query_planner() API change

From: Atri Sharma <atri(dot)jiit(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: query_planner() API change
Date: 2013-08-05 17:24:41
Message-ID: CAOeZVie=ZDzCStSd8+Oh7bq7B+Z_bNJG6=Z5pYhFrY1sei55Dg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 5, 2013 at 6:19 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Atri Sharma <atri(dot)jiit(at)gmail(dot)com> writes:
>>> While we could complicate query_planner()'s API even more to add some
>>> understanding of unnecessary resjunk items, I think this is probably
>>> the straw that breaks the camel's back for the current approach here.
>>> There is already a comment like this in query_planner():
>>>
>>> * This introduces some undesirable coupling between this code and
>>> * grouping_planner, but the alternatives seem even uglier; we couldn't
>>> * pass back completed paths without making these decisions here.
>
>> I agree with the idea,but am trying to understand why adding
>> understanding of resjunk columns is a bad idea. Just for understanding
>> purpose, could you please elaborate a bit on it?
>
> It's just that doing it that way would require making both planner.c and
> planmain.c intimately involved in the decision about whether suppressing
> resjunk ORDER BY targets is a win. Really, anything to do with
> ordering/grouping implementation decisions is grouping_planner's business.
> So putting chunks of that logic in a completely different file doesn't
> seem like a great design, especially not if it requires weighing down
> query_planner()'s API even more. query_planner should only be concerned
> with scan/join planning.
>
> Basically, we'd be moving knowledge of how to dig the best paths out of a
> RelOptInfo from query_planner to grouping_planner --- which when you think
> about it seems like mostly a wash from a modularity standpoint, anyway.
> Having done that, we can get query_planner's fingers out of a number of
> issues that are really grouping_planner's business. Returning the
> RelOptInfo also eliminates the baked-into-the-API assumption that only one
> of the presorted path(s) could be of interest to grouping_planner, which
> is something I've long suspected would become a problem someday.
>
> On balance I'm feeling like this is a win even without considering the
> proposed changes for resjunk targets.

Thanks a ton for such a detailed explanation.

So, query_planner() returns both,the unsorted and presorted paths and
lets grouping_planner() decide between them, and grouping_planner()
ignores unnecessary ORDER BY columns,right?

Sorry if I am being naive here, I am just trying to assimilate the
overall process for my understanding.

Thanks,

atri

--
Regards,

Atri
l'apprenant

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2013-08-05 17:39:31 Re: Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])
Previous Message Josh Berkus 2013-08-05 17:21:56 Re: File-per-GUC WAS: Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])