Re: patch: function xmltable

Andres Freund wrote:
> Hi,
> 
> On 2017-01-24 17:38:49 -0300, Alvaro Herrera wrote:
> > +static Datum ExecEvalTableExpr(TableExprState *tstate, ExprContext *econtext,
> > +				  bool *isnull);
> > +static Datum ExecEvalTableExprFast(TableExprState *exprstate, ExprContext *econtext,
> > +					  bool *isNull);
> > +static Datum tabexprFetchRow(TableExprState *tstate, ExprContext *econtext,
> > +				bool *isNull);
> > +static void tabexprInitialize(TableExprState *tstate, ExprContext *econtext,
> > +				  Datum doc);
> > +static void ShutdownTableExpr(Datum arg);
> 
> To me this (and a lot of the other code) hints quite strongly that
> expression evalution is the wrong approach to implementing this.  What
> you're essentially doing is building a vulcano style scan node.  Even if
> we can this, we shouldn't double down on the bad decision to have these
> magic expressions that return multiple rows.  There's historical reason
> for tSRFs, but we shouldn't add more weirdness like this.

Thanks for giving it a look.  I have long thought that this patch would
be at odds with your overall executor work.

XMLTABLE is specified by the standard to return multiple rows ... but
then as far as my reading goes, it is only supposed to be supported in
the range table (FROM clause) not in the target list.  I wonder if
this would end up better if we only tried to support it in RT.  I asked
Pavel to implement it like that a few weeks ago, but ...

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services