Andres Freund wrote: > On 2017-01-16 12:17:46 -0300, Alvaro Herrera wrote: > > Andres Freund wrote: > > > > > That worked quite well. So we have a few questions, before I clean this > > > up: > > > > > > - For now the node is named 'Srf' both internally and in explain - not > > > sure if we want to make that something longer/easier to understand for > > > others? Proposals? TargetFunctionScan? SetResult? > > > > > > - We could alternatively add all this into the Result node - it's not > > > actually a lot of new code, and most of that is boilerplate stuff > > > about adding a new node. I'm ok with both. > > > > Hmm. I wonder if your stuff could be used as support code for > > XMLTABLE[1]. > > I don't immediately see what functionality overlaps, could you expand on > that?
Well, I haven't read any previous patches in this area, but the xmltable patch adds a new way of handling set-returning expressions, so it appears vaguely related. These aren't properly functions in the current sense of the word, though. There is some parallel to what ExecMakeFunctionResult does, which I suppose is related. > > Currently it has a bit of additional code of its own, > > though admittedly it's very little code executor-side. Would you mind > > sharing a patch, or more details on how it works? > > Can do both; cleaning up the patch now. What we're talking about here is > a way to implement targetlist SRF that is based on: > > 1) a patch by Tom that creates additional Result (or now Srf) executor > nodes containing SRF evaluation. This guarantees that only Result/Srf > nodes have to deal with targetlist SRF evaluation. > > 2) new code to evaluate SRFs in the new Result/Srf node, that doesn't > rely on ExecEvalExpr et al. to have a IsDone argument. Instead > there's special code to handle that in the new node. That's possible > because it's now guaranted that all SRFs are "toplevel" in the > relevant targetlist(s). > > 3) Removal of all nearly tSRF related code execQual.c and other > executor/ files, including the node->ps.ps_TupFromTlist checks > everywhere. > > makes sense? Hmm, okay. (The ps_TupFromTlist thing has long seemed an ugly construction.) I think the current term for this kind of thing is TableFunction -- are you really naming this "Srf" literally? It seems strange, but maybe it's just me. > > Nested targetlist SRFs make my head spin. I suppose they may have some > > use, but where would you really want this: > > I think there's some cases where it can be useful. Targetlist SRFs as a > whole really are much more about backward compatibility than anything :) Sure. > > ? If supporting the former makes it harder to support/optimize more > > reasonable cases, it seems fair game to leave them behind. > > I don't want to desupport them, just don't want to restructure (one node > doing several levels of SRFs, instead of one per level) just to make it > easier to give good estimates. No objections. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
