Re: WIP: Upper planner pathification - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: WIP: Upper planner pathification |
Date | |
Msg-id | 18447.1458312246@sss.pgh.pa.us Whole thread Raw |
In response to | Re: WIP: Upper planner pathification (Kouhei Kaigai <kaigai@ak.jp.nec.com>) |
Responses |
Re: WIP: Upper planner pathification
|
List | pgsql-hackers |
Kouhei Kaigai <kaigai@ak.jp.nec.com> writes: > On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I do not, however, like the proposal to expose wflists and so forth. >> Those are internal data structures in grouping_planner and I absolutely >> refuse to promise that they're going to stay stable. > Hmm... It's not easy to imagine a case that extension wants own idea > to extract window functions from the target list and select active > windows, even if extension wants to have own executor and own cost > estimation logic. > In case when extension tries to add WindowPath + CustomPath(Sort), > extension is interested in alternative sort task, but not window > function itself. It is natural to follow the built-in implementation, > thus, it motivates extension author to take copy & paste the code. > select_active_windows() is static, so extension needs to have same > routine on their side. Well, to be perfectly blunt about it, I have said from day one that this notion that a CustomScan extension will be able to cause arbitrary planner behavior changes is loony. We are simply not going to drop a hook into every tenth line of the planner for you, nor de-static-ify every internal function, nor (almost equivalently) expose the data structures those functions produce, because it would cripple core planner development to try to keep the implied APIs stable. And I continue to maintain that any actually-generally-useful ideas would be better handled by submitting them as patches to the core planner, rather than trying to implement them in an arms-length extension. In the case at hand, I notice that the WindowFuncLists struct is actually from find_window_functions in clauses.c, so an extension that needed to get hold of that would be unlikely to do any copying and pasting anyhow -- it'd just call find_window_functions again. (That only needs to search the targetlist, so it's not that expensive.) The other lists you mention are all tightly tied to specific, and not terribly well-designed, implementation strategies for grouping sets and window functions. Those are *very* likely to change in the near future; and even if they don't, I'm skeptical that an external implementor of grouping sets or window functions would want to use exactly those same implementation strategies. Maybe the only reason you're there at all is you want to be smarter about the order of doing window functions, for example. So I really don't want to export any of that stuff. As for other details of the proposed patch, I was intending to put all the hook calls into grouping_planner for consistency, rather than scattering them between grouping_planner and its subroutines. So that would probably mean calling the hook for a given step *before* we generate the core paths for that step, not after. Did you have a reason to want the other order? (If you say "so the hook can look at the core-made paths", I'm going to say "that's a bad idea". It'd further increase the coupling between a CustomScan extension and core.) regards, tom lane
pgsql-hackers by date: