Re: upper planner path-ification - Mailing list pgsql-hackers

From Tom Lane
Subject Re: upper planner path-ification
Date
Msg-id 30117.1431876866@sss.pgh.pa.us
Whole thread Raw
In response to Re: upper planner path-ification  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
Responses Re: upper planner path-ification  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
List pgsql-hackers
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Robert" == Robert Haas <robertmhaas@gmail.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



pgsql-hackers by date:

Previous
From: Andrew Gierth
Date:
Subject: Re: upper planner path-ification
Next
From: Dmitry Dolgov
Date:
Subject: Re: jsonb concatenate operator's semantics seem questionable