Skip site navigation (1) Skip section navigation (2)

Re: patch: function xmltable

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Craig Ringer <craig(at)2ndquadrant(dot)com>
Subject: Re: patch: function xmltable
Date: 2017-03-03 09:28:54
Message-ID: CAFj8pRCfSUihcWts78_HmcTFnAfgeCk304wZ6w+k+ADt=eQt5A@mail.gmail.com (view raw, whole thread or download thread mbox)
Thread:
Lists: pgsql-hackers
2017-03-02 22:35 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> Pavel Stehule wrote:
> > 2017-03-02 19:32 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:
> >
> > > So in the old (non-executor-node) implementation, you could attach WITH
> > > ORDINALITY to the xmltable expression and it would count the output
> > > rows, regardless of which XML document it comes from.  With the new
> > > implementation, the grammar no longer accepts it.  To count output
> rows,
> > > you still need to use row_number().  Maybe this is okay.  This is the
> > > example from the docs, and I add another XML document with two more
> rows
> > > for xmltable.  Look at the three numbering columns ...
> > >
> >
> > It is expected - now tablefunc are not special case of SRF, so it lost
> all
> > SRF functionality. It is not critical lost - it supports internally FOR
> > ORDINALITY column, and classic ROW_NUMBER can be used. It can be enhanced
> > to support WITH ORDINALITY in future, but I have not any use case for it.
>
> Fine.
>
> After looking at the new executor code a bit, I noticed that we don't
> need the resultSlot anymore; we can use the ss_ScanTupleSlot instead.
> Because resultSlot was being used in the xml.c code (which already
> appeared a bit dubious to me), I changed the interface so that instead
> the things that it read from it are passed as parameters -- namely, in
> InitBuilder we pass natts, and in GetValue we pass typid and typmod.
>

I had similar feeling

>
> Secondly, I noticed we have the FetchRow routine produce a minimal
> tuple, put it in a slot; then its caller takes the slot and put the
> tuple in the tuplestore.  This is pointless; we can just have FetchRow
> put the tuple in the tuplestore directly and not bother with any slot
> manipulations there.  This simplifies the code a bit.
>
>
has sense

attached update with fixed tests

Regards

Pavel



> Here's v47 with those changes.
>
> --
> Álvaro Herrera                https://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Attachment: xmltable-48.patch.gz
Description: application/x-gzip (37.4 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Amit KapilaDate: 2017-03-03 09:42:52
Subject: Re: [BUG FIX] Removing NamedLWLockTrancheArray
Previous:From: Kyotaro HORIGUCHIDate: 2017-03-03 08:31:19
Subject: Re: [BUG FIX] Removing NamedLWLockTrancheArray

Privacy Policy | About PostgreSQL
Copyright © 1996-2017 The PostgreSQL Global Development Group