From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)
Date: Mon, 16 Jan 2017 16:04:34 -0300
Message-ID: <20170116190434.7xcy7oxpubfw7mys@alvherre.pgsql> (raw)
In-Reply-To: <20170116184417.esbyb6lzkotpibam@alap3.anarazel.de>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, andres@anarazel.de, tgl@sss.pgh.pa.us
Subject: Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)
In-Reply-To: <20170116190434.7xcy7oxpubfw7mys@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox