On 2016-09-12 17:36:07 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2016-09-12 13:26:20 -0400, Tom Lane wrote: > >> Stepping back a little bit ... way back at the start of this thread > >> you muttered about possibly implementing tSRFs as a special pipeline > >> node type, a la Result. That would have the same benefits in terms > >> of being able to take SRF support out of the main execQual code paths. > >> I, and I think some other people, felt that the LATERAL approach would > >> be a cleaner answer --- but now that we're seeing some of the messy > >> details required to make the LATERAL way work, I'm beginning to have > >> second thoughts. I wonder if we should do at least a POC implementation > >> of the other way to get a better fix on which way is really cleaner. > > > I'm not particularly in love in restarting with a different approach. I > > think fixing the ROWS FROM expansion is the only really painful bit, and > > that seems like it's independently beneficial to allow for suppression > > of expansion there. > > Um, I dunno. You've added half a thousand lines of not-highly-readable- > nor-extensively-commented code to the planner; that certainly reaches *my* > threshold of pain.
Well, I certainly plan (and started to) make that code easier to understand, and better commented. It also removes ~1400 LOC of not easy to understand code... A good chunk of that'd would also be removed with a Result style approach, but far from all. > I'm also growing rather concerned that the LATERAL > approach is going to lock us into some unremovable incompatibilities > no matter how much we might regret that later (and in view of how quickly > I got my wrist slapped in <001201d18524$f84c4580$e8e4d080$@pcorp.us>, > I am afraid there may be more pushback awaiting us than we think). I don't think it'd be all that hard to add something like the current LCM behaviour into nodeFunctionscan.c if we really wanted. But I think it'll be better to just say no here. > If we go with a Result-like tSRF evaluation node, then whether we change > semantics or not becomes mostly a matter of what that node does. It could > become basically a wrapper around the existing ExecTargetList() logic if > we needed to provide backwards-compatible behavior. As you previously objected: If we keep ExecTargetList() style logic, we need to keep most of execQual.c's handling of ExprMultipleResult et al, and that's going to prevent the stuff I want to work on. Because handling ExprMultipleResult in all these places is a serious issue WRT making expression evaluation faster. If we find a good answer to that, I'd be more open to pursuing this approach. > > I actually had started to work on a Result style approach, and I don't > > think it turned out that nice. But I didn't complete it, so I might just > > be wrong. > > It's also possible that it's easier now in the post-pathification code > base than it was before. After contemplating my navel for awhile, it > seems like the core of the planner code for a Result-like approach would > be something very close to make_group_input_target(), plus a thing like > pull_var_clause() that extracts SRFs rather than Vars. Those two > functions, including their lengthy comments, weigh in at ~250 lines. > Admittedly there'd be some boilerplate on top of that, if we invent a > new plan node type rather than extending Result, but not all that much. That's pretty much what I did (or rather started to do), yes. I had something that was called from grouping_planner() that added the new node ontop of the original set of paths, after grouping or after distinct, depending on where SRFs were referenced. The biggest benefit I saw with that is that there's no need to push things into a subquery, and that the ordering is a lot more explicit. > If you like, I'll have a go at drafting a patch in that style. Do you > happen to still have the executor side of what you did, so I don't have > to reinvent that? The executor side is actually what I found harder here. Either we end up keeping most of ExecQual's handling, or we reinvent a good deal of separate logic. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
