Re: [HACKERS] patch: function xmltable - Mailing list pgsql-hackers
| From | Pavel Stehule |
|---|---|
| Subject | Re: [HACKERS] patch: function xmltable |
| Date | |
| Msg-id | CAFj8pRBi7_Gne=mSMikZ8S7SV61myLLszfetUqFkhHpotnZ6QA@mail.gmail.com Whole thread Raw |
| In response to | Re: [HACKERS] patch: function xmltable (Andres Freund <andres@anarazel.de>) |
| Responses |
Re: [HACKERS] patch: function xmltable
|
| List | pgsql-hackers |
Hi
2017-01-25 1:35 GMT+01:00 Andres Freund <andres@anarazel.de>:
On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote:
> 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.
Not fundamentally, but it makes it harder.
If you plan to hold support SRFin target list, then nothing is different. In last patch is executed under nodeProjectSet.
> 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 ...
Right - it makes sense in the FROM list - but then it should be an
executor node, instead of some expression thingy.
The XMLTABLE function is from user perspective, from implementation perspective a form of SRF function. I use own executor node, because fcinfo is complex already and not too enough to hold all information about result columns.
The implementation as RT doesn't reduce code - it is just moving to different file.
I'll try to explain my motivation. Please, check it and correct me if I am wrong. I don't keep on my implementation - just try to implement XMLTABLE be consistent with another behave and be used all time without any surprise.
1. Any function that produces a content can be used in target list. We support SRF in target list and in FROM part. Why XMLTABLE should be a exception?
2. In standard the XMLTABLE is placed only on FROM part - but standard doesn't need to solve my question - there are not SRF functions allowed in targetlist.
If there be a common decision so this inconsistency (in behave of this kind of functions) is expected, required - then I have not a problem to remove this support from XMLTABLE.
In this moment I don't see a technical reason for this step - with last Andres changes the support of XMLTABLE in target list needs less than 40 lines and there is not any special path for XMLTABLE only. Andres write support for SRF functions and SRF operator. TableExpr is third category.
Regards
Pavel
Greetings,
Andres Freund
pgsql-hackers by date: