Re: upper planner path-ification

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers\(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: upper planner path-ification
Date: 2015-05-17 15:34:26
Message-ID: 30117.1431876866@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> "Robert" == Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Robert> I think grouping_planner() is badly in need of some refactoring
> Robert> just to make it shorter. It's over 1000 lines of code, which
> Robert> IMHO is a fairly ridiculous length for a single function.

> If there's interest, we could do that specific task as part of adding
> hashagg support for grouping sets (which would otherwise make it even
> longer), or as preparatory work for that.

I think that refactoring without changing anything about the way it works
will be painful and probably ultimately a dead end. As an example, the
current_pathkeys local variable is state that's needed throughout that
process, so you'd need some messy notation to pass it around (unless you
stuck it into PlannerRoot, which would be ugly too). But that would
go away in a path-ified universe, because each Path would be marked as
to its output sort order. More, a lot of the code that you'd be
relocating is code that we should be trying to get rid of altogether,
not just relocate (to wit all the stuff that does cost-based comparisons
of alternatives).

So I'm all for refactoring, but I think it will happen as a natural
byproduct of path-ification, and otherwise would be rather forced.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2015-05-17 15:37:23 Re: jsonb concatenate operator's semantics seem questionable
Previous Message Andrew Gierth 2015-05-17 15:14:29 Re: upper planner path-ification