Andres Freund <and...@anarazel.de> writes: > On 2016-09-12 17:36:07 -0400, Tom Lane wrote: >> 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. Hm, I've not studied 0006 yet, but surely that's executor code that would go away with *any* approach that takes away the need for generic execQual to support SRFs? I don't see that it counts while discussing which way we take to reach that point. >> 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 <firstname.lastname@example.org>, >> 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. "Just say no" soon translates to memes about "disasters like the removal of implicit casting" (which in fact is not what 8.3 did, but I've grown pretty damn tired of the amount of bitching that that cleanup did and still does provoke). In any case, it feels like the LATERAL approach is locking us into more and subtler incompatibilities than just that one. >> 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. You're inventing objections. It won't require that any more than the LATERAL approach does; it's basically the same code as whatever nodeFunctionscan is going to do, but packaged as a pipeline eval node rather than a base scan node. Or to be clearer: what I'm suggesting it would contain is ExecTargetList's logic about restarting individual SRFs. That wouldn't propagate into execQual because we would only allow SRFs at the top level of the node's tlist, just like nodeFunctionscan does. ExecMakeTableFunctionResult doesn't require the generic execQual code to support SRFs today, and it still wouldn't. In larger terms: the whole point here is to fish SRF calls up to the top level of the tlist of whatever node is executing them, where they can be special-cased by that node. Their SRF-free argument expressions would be evaluated by generic execQual. AFAICS this goes through in the same way from the executor's viewpoint whether we use LATERAL as the query restructuring method or a SRF-capable variant of Result. But it's now looking to me like the latter would be a lot simpler from the point of view of planner complexity, and in particular from the point of view of proving correctness (equivalence of the query transformation). regards, tom lane -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers