Re: [HACKERS] patch: function xmltable

2017-03-08 Thread Alvaro Herrera
Pavel Stehule wrote: > 2017-03-08 17:32 GMT+01:00 Alvaro Herrera : > > > For daily work the default schema support is much more interesting. > > > > Let's see that one, then. It was part of the original submission so > > depending on how the patch we looks can still

Re: [HACKERS] patch: function xmltable

2017-03-08 Thread Pavel Stehule
2017-03-08 17:32 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2017-03-08 17:01 GMT+01:00 Alvaro Herrera : > > > > I didn't add the change you proposed here to keep COLUMNS optional; > > > instead, I just made COLUMNS mandatory. I think

Re: [HACKERS] patch: function xmltable

2017-03-08 Thread Alvaro Herrera
Pavel Stehule wrote: > 2017-03-08 17:01 GMT+01:00 Alvaro Herrera : > > I didn't add the change you proposed here to keep COLUMNS optional; > > instead, I just made COLUMNS mandatory. I think what you propose here > > is not entirely out of the question, but you left out

Re: [HACKERS] patch: function xmltable

2017-03-08 Thread Pavel Stehule
2017-03-08 17:01 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > Hi > > > > I used your idea about special columns when COLUMNS are not explicitly > > defined. > > > > All lines that you are dislike removed. > > I just pushed XMLTABLE, after some additional

Re: [HACKERS] patch: function xmltable

2017-03-08 Thread Alvaro Herrera
Pavel Stehule wrote: > Hi > > I used your idea about special columns when COLUMNS are not explicitly > defined. > > All lines that you are dislike removed. I just pushed XMLTABLE, after some additional changes. Please test it thoroughly and report any problems. I didn't add the change you

Re: [HACKERS] patch: function xmltable

2017-03-05 Thread Pavel Stehule
Hi I used your idea about special columns when COLUMNS are not explicitly defined. All lines that you are dislike removed. Now, almost all code, related to this behave, is in next few lines. + /* +* Use implicit column when it is necessary. The COLUMNS clause is optional +* on Oracle

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Pavel Stehule
2017-03-03 21:04 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2017-03-03 19:15 GMT+01:00 Alvaro Herrera : > > > > 2. As I've complained many times, I find the way we manage an empty > > > COLUMNS clause pretty bad. The standard doesn't

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Alvaro Herrera
Pavel Stehule wrote: > 2017-03-03 19:15 GMT+01:00 Alvaro Herrera : > > 2. As I've complained many times, I find the way we manage an empty > > COLUMNS clause pretty bad. The standard doesn't require that syntax > > (COLUMNS is required), and I don't like the

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Pavel Stehule
2017-03-03 19:42 GMT+01:00 Pavel Stehule : > > > 2017-03-03 19:15 GMT+01:00 Alvaro Herrera : > >> Pavel Stehule wrote: >> >> > attached update with fixed tests >> >> Heh, I noticed that you removed the libxml "context" lines that >> differentiate

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Pavel Stehule
2017-03-03 19:15 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > > attached update with fixed tests > > Heh, I noticed that you removed the libxml "context" lines that > differentiate xml.out from xml_2.out when doing this. My implementation > emits those lines,

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Alvaro Herrera
Pavel Stehule wrote: > attached update with fixed tests Heh, I noticed that you removed the libxml "context" lines that differentiate xml.out from xml_2.out when doing this. My implementation emits those lines, so it was failing for me. I restored them. I also changed a few things to avoid

Re: [HACKERS] patch: function xmltable

2017-03-03 Thread Pavel Stehule
2017-03-02 22:35 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2017-03-02 19:32 GMT+01:00 Alvaro Herrera : > > > > > So in the old (non-executor-node) implementation, you could attach WITH > > > ORDINALITY to the xmltable expression and

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Alvaro Herrera
Pavel Stehule wrote: > 2017-03-02 19:32 GMT+01:00 Alvaro Herrera : > > > 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

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Pavel Stehule
2017-03-02 19:32 GMT+01:00 Alvaro Herrera : > 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 >

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Alvaro Herrera
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

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Pavel Stehule
Dne 2. 3. 2017 18:14 napsal uživatel "Alvaro Herrera" < alvhe...@2ndquadrant.com>: Pavel Stehule wrote: > It is documented already > > "If the PATH matches an empty tag the result is an empty string" Hmm, okay. But what we have here is not an empty tag, but a tag that is completely missing. I

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Alvaro Herrera
Pavel Stehule wrote: > It is documented already > > "If the PATH matches an empty tag the result is an empty string" Hmm, okay. But what we have here is not an empty tag, but a tag that is completely missing. I don't think those two cases should be treated in the same way ... > Attached new

Re: [HACKERS] patch: function xmltable

2017-03-02 Thread Pavel Stehule
2017-03-02 8:04 GMT+01:00 Pavel Stehule : > Hi > > 2017-03-02 1:12 GMT+01:00 Alvaro Herrera : > >> >> I've been giving this a look. I started by tweaking the docs once >> again, and while verifying that the example works as expected, I >>

Re: [HACKERS] patch: function xmltable

2017-03-01 Thread Pavel Stehule
Hi 2017-03-02 1:12 GMT+01:00 Alvaro Herrera : > > I've been giving this a look. I started by tweaking the docs once > again, and while verifying that the example works as expected, I > replayed what I have in sgml: > > ... begin SGML paste ... > > For

Re: [HACKERS] patch: function xmltable

2017-03-01 Thread Alvaro Herrera
I've been giving this a look. I started by tweaking the docs once again, and while verifying that the example works as expected, I replayed what I have in sgml: ... begin SGML paste ... For example, given the following XML document: the following query produces the result

Re: [HACKERS] patch: function xmltable

2017-01-31 Thread Pavel Stehule
Hi 2017-01-31 14:57 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2017-01-24 21:38 GMT+01:00 Alvaro Herrera : > > > > I think it would be good to have a more complex test case in regress -- > > > let's say there is a table with some

Re: [HACKERS] patch: function xmltable

2017-01-31 Thread Pavel Stehule
2017-01-31 14:57 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2017-01-24 21:38 GMT+01:00 Alvaro Herrera : > > > > I think it would be good to have a more complex test case in regress -- > > > let's say there is a table with some simple

Re: [HACKERS] patch: function xmltable

2017-01-31 Thread Alvaro Herrera
Pavel Stehule wrote: > 2017-01-24 21:38 GMT+01:00 Alvaro Herrera : > > I think it would be good to have a more complex test case in regress -- > > let's say there is a table with some simple XML values, then we use > > XMLFOREST (or maybe one of the table_to_xml

Re: [HACKERS] patch: function xmltable

2017-01-31 Thread Pavel Stehule
2017-01-24 21:38 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > > * SELECT (xmltable(..)).* + regress tests > > * compilation and regress tests without --with-libxml > > Thanks. I just realized that this is doing more work than necessary -- > I think it would be

Re: [HACKERS] patch: function xmltable

2017-01-30 Thread Michael Paquier
On Tue, Jan 31, 2017 at 5:18 AM, Pavel Stehule wrote: > true, I am sorry Last status is a new patch and no reviews. On top of that this thread is quite active. So moved to next CF. Pavel, please be careful about the status of the patch on the CF app, it was set to

Re: [HACKERS] patch: function xmltable

2017-01-30 Thread Pavel Stehule
Hi 2017-01-30 20:38 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > > I am sending new version - it is based on own executor scan node and > > tuplestore. > > > > Some now obsolete regress tests removed, some new added. > > > > The executor code (memory context

Re: [HACKERS] patch: function xmltable

2017-01-30 Thread Alvaro Herrera
Pavel Stehule wrote: > I am sending new version - it is based on own executor scan node and > tuplestore. > > Some now obsolete regress tests removed, some new added. > > The executor code (memory context usage) should be cleaned little bit - but > other code should be good. I think you forgot

Re: [HACKERS] patch: function xmltable

2017-01-30 Thread Pavel Stehule
Hi 2017-01-25 15:07 GMT+01:00 Alvaro Herrera : > Tom Lane wrote: > > Andres Freund writes: > > > On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote: > > >> XMLTABLE is specified by the standard to return multiple rows ... but > > >> then as far as

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Pavel Stehule
2017-01-25 15:07 GMT+01:00 Alvaro Herrera : > Tom Lane wrote: > > Andres Freund writes: > > > On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote: > > >> XMLTABLE is specified by the standard to return multiple rows ... but > > >> then as far as my

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Pavel Stehule
2017-01-25 23:33 GMT+01:00 Andres Freund : > On 2017-01-25 22:51:37 +0100, Pavel Stehule wrote: > > 2017-01-25 22:40 GMT+01:00 Andres Freund : > > > > I afraid when I cannot to reuse a SRF infrastructure, I have to > > > reimplement > > > > it partially :(

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Andres Freund
On 2017-01-25 22:51:37 +0100, Pavel Stehule wrote: > 2017-01-25 22:40 GMT+01:00 Andres Freund : > > > I afraid when I cannot to reuse a SRF infrastructure, I have to > > reimplement > > > it partially :( - mainly for usage in "ROWS FROM ()" > > > > The TableExpr implementation

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Pavel Stehule
2017-01-25 22:40 GMT+01:00 Andres Freund : > Hi, > > > > > 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

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Andres Freund
Hi, > > > 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

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Pavel Stehule
> > > > > > > If you plan to hold support SRFin target list, then nothing is different. > > In last patch is executed under nodeProjectSet. > > It is, because we suddenly need to call different functions - and I'm > revamping most of execQual to have an opcode dispatch based execution > model

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Andres Freund
Hi, On 2017-01-25 05:45:24 +0100, Pavel Stehule wrote: > 2017-01-25 1:35 GMT+01:00 Andres Freund : > > > 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: > > > > >

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Alvaro Herrera
Tom Lane wrote: > Andres Freund writes: > > On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote: > >> 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

Re: [HACKERS] patch: function xmltable

2017-01-25 Thread Pavel Stehule
2017-01-24 21:38 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > > * SELECT (xmltable(..)).* + regress tests > > * compilation and regress tests without --with-libxml > > Thanks. I just realized that this is doing more work than necessary -- > ?? I don't

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Pavel Stehule
2017-01-25 5:45 GMT+01:00 Tom Lane : > Andres Freund writes: > > On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote: > >> XMLTABLE is specified by the standard to return multiple rows ... but > >> then as far as my reading goes, it is only supposed to be

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Pavel Stehule
Hi 2017-01-25 1:35 GMT+01:00 Andres Freund : > 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 >

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Tom Lane
Andres Freund writes: > On 2017-01-24 21:32:56 -0300, Alvaro Herrera wrote: >> 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

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Andres Freund
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

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Alvaro Herrera
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,

Re: [HACKERS] patch: function xmltable

2017-01-24 Thread Andres Freund
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, > +

Re: [HACKERS] patch: function xmltable

2017-01-19 Thread Pavel Stehule
2017-01-19 13:35 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > Hi > > > > New update - rebase after yesterday changes. > > > > What you want to change? > > I think the problem might come from the still pending patch on that > series, which Andres posted in >

Re: [HACKERS] patch: function xmltable

2017-01-19 Thread Alvaro Herrera
Pavel Stehule wrote: > Hi > > New update - rebase after yesterday changes. > > What you want to change? I think the problem might come from the still pending patch on that series, which Andres posted in https://www.postgresql.org/message-id/20170118221154.aldebi7yyjvds...@alap3.anarazel.de As

Re: [HACKERS] patch: function xmltable

2017-01-17 Thread Pavel Stehule
2017-01-16 23:51 GMT+01:00 Alvaro Herrera : > Given > https://www.postgresql.org/message-id/20170116210019. > a3glfwspg5lnf...@alap3.anarazel.de > which is going to heavily change how the executor works in this area, I > am returning this patch to you again. I would

Re: [HACKERS] patch: function xmltable

2017-01-16 Thread Alvaro Herrera
In case this still matters, I think GetValue should look more or less like this (untested): /* * Return the value for column number 'colnum' for the current row. If column * -1 is requested, return representation of the whole row. * * This leaks memory, so be sure to reset often the context

Re: [HACKERS] patch: function xmltable

2017-01-16 Thread Alvaro Herrera
Given https://www.postgresql.org/message-id/20170116210019.a3glfwspg5lnf...@alap3.anarazel.de which is going to heavily change how the executor works in this area, I am returning this patch to you again. I would like a few rather minor changes: 1. to_xmlstr can be replaced with calls to

Re: [HACKERS] patch: function xmltable

2017-01-16 Thread Alvaro Herrera
I just realized that your new xml_xmlnodetostr is line-by-line identical to the existing xml_xmlnodetoxmltype except for two or three lines. I think that's wrong. I'm going to patch the existing function so that they can share code. -- Álvaro Herrerahttps://www.2ndQuadrant.com/

Re: [HACKERS] patch: function xmltable

2017-01-11 Thread Alvaro Herrera
Pavel Stehule wrote: > another update - lot of cleaning Thanks. The more I look at this, the less I like using NameArgExpr for namespaces. It looks all wrong to me, and it causes ugly code all over. Maybe I just need to look at it a bit longer. -- Álvaro Herrera

Re: [HACKERS] patch: function xmltable

2016-12-22 Thread Alvaro Herrera
Pavel Stehule wrote: > another update - lot of cleaning Ah, the tupledesc stuff in this one appears much more reasonable to me. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers

Re: [HACKERS] patch: function xmltable

2016-12-07 Thread Pavel Stehule
2016-12-07 20:50 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2016-12-07 18:34 GMT+01:00 Alvaro Herrera : > > > > Hmm. Now that I see how this works, by having the GetValue "guess" > what > > > is going on and have a special case for

Re: [HACKERS] patch: function xmltable

2016-12-07 Thread Alvaro Herrera
Pavel Stehule wrote: > 2016-12-07 18:34 GMT+01:00 Alvaro Herrera : > > Hmm. Now that I see how this works, by having the GetValue "guess" what > > is going on and have a special case for it, I actually don't like it > > very much. It seems way too magical. I think we

Re: [HACKERS] patch: function xmltable

2016-12-07 Thread Alvaro Herrera
Pavel Stehule wrote: > I fixed two issues. > > 2. there was reverse setting in NOT NULL flag Ah-hah, that was silly, thanks. > 1. there are not columns data when there are not any explicit column - fixed Hmm. Now that I see how this works, by having the GetValue "guess" what is going on and

Re: [HACKERS] patch: function xmltable

2016-12-04 Thread Alvaro Herrera
Pavel Stehule wrote: > 2016-12-04 23:00 GMT+01:00 Pavel Stehule : > > I am not sure if I understand well to your ideas - please, check attached > > patch. Thanks, that's what I meant, but I think you went a bit overboard creating new functions in execQual -- seems to me

Re: [HACKERS] patch: function xmltable

2016-12-03 Thread Alvaro Herrera
Pavel Stehule wrote: > 2016-12-02 23:25 GMT+01:00 Alvaro Herrera : > > This is looking much better now, but it still needs at least the > > following changes. > > > > First, we need to fix is the per_rowset_memcxt thingy. I think the way > > it's currently being used

Re: [HACKERS] patch: function xmltable

2016-12-02 Thread Pavel Stehule
Hi 2016-12-02 23:25 GMT+01:00 Alvaro Herrera : > Here's version 17. I have made significant changes here. > > 1. Restructure the execQual code. Instead of a PG_TRY wrapper, I have > split this code in three pieces; there's the main code with the PG_TRY > wrappers and

Re: [HACKERS] patch: function xmltable

2016-12-02 Thread Alvaro Herrera
Hm, you omitted tableexpr.h from the v15 patch ... -- Álvaro Herrerahttps://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:

Re: [HACKERS] patch: function xmltable

2016-12-01 Thread Haribabu Kommi
On Thu, Dec 1, 2016 at 2:21 AM, Pavel Stehule wrote: > Dne 30. 11. 2016 14:53 napsal uživatel "Pavel Stehule" < > pavel.steh...@gmail.com>: > > > > > > > > 2016-11-30 13:38 GMT+01:00 Alvaro Herrera : > >> > >> Pavel Stehule wrote: > >> >

Re: [HACKERS] patch: function xmltable

2016-11-30 Thread Pavel Stehule
Dne 30. 11. 2016 14:53 napsal uživatel "Pavel Stehule" < pavel.steh...@gmail.com>: > > > > 2016-11-30 13:38 GMT+01:00 Alvaro Herrera : >> >> Pavel Stehule wrote: >> > 2016-11-30 2:40 GMT+01:00 Craig Ringer : >> > >> > > On 30 November 2016 at 05:36,

Re: [HACKERS] patch: function xmltable

2016-11-30 Thread Pavel Stehule
2016-11-30 13:38 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > 2016-11-30 2:40 GMT+01:00 Craig Ringer : > > > > > On 30 November 2016 at 05:36, Pavel Stehule > > > wrote: > > > > > > > The problem is in unreserved

Re: [HACKERS] patch: function xmltable

2016-11-30 Thread Alvaro Herrera
Pavel Stehule wrote: > 2016-11-30 2:40 GMT+01:00 Craig Ringer : > > > On 30 November 2016 at 05:36, Pavel Stehule > > wrote: > > > > > The problem is in unreserved keyword "PASSING" probably. > > > > Yeah, I think that's what I hit when trying to

Re: [HACKERS] patch: function xmltable

2016-11-29 Thread Pavel Stehule
2016-11-30 2:40 GMT+01:00 Craig Ringer : > On 30 November 2016 at 05:36, Pavel Stehule > wrote: > > > The problem is in unreserved keyword "PASSING" probably. > > Yeah, I think that's what I hit when trying to change it. > > Can't you just

Re: [HACKERS] patch: function xmltable

2016-11-29 Thread Craig Ringer
On 30 November 2016 at 05:36, Pavel Stehule wrote: > The problem is in unreserved keyword "PASSING" probably. Yeah, I think that's what I hit when trying to change it. Can't you just parenthesize the expression to use operators like || etc? If so, not a big deal.

Re: [HACKERS] patch: function xmltable

2016-11-28 Thread Alvaro Herrera
Pavel Stehule wrote: > Here is updated patch without default namespace support (and without XPath > expression transformation). > > Due last changes in parser > https://github.com/postgres/postgres/commit/906bfcad7ba7cb3863fe0e2a7810be8e3cd84fbd > I had to use c_expr on other positions (

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Pavel Stehule
2016-11-25 7:44 GMT+01:00 Pavel Stehule : > > > 2016-11-25 3:31 GMT+01:00 Alvaro Herrera : > >> Michael Paquier wrote: >> >> > Nit: I did not look at the patch in details, >> > but I find the size of the latest version sent, 167kB, scary as it >>

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Pavel Stehule
2016-11-25 3:31 GMT+01:00 Alvaro Herrera : > Michael Paquier wrote: > > > Nit: I did not look at the patch in details, > > but I find the size of the latest version sent, 167kB, scary as it > > complicates review and increases the likeliness of bugs. > > Here's the stat.

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Alvaro Herrera
Michael Paquier wrote: > Nit: I did not look at the patch in details, > but I find the size of the latest version sent, 167kB, scary as it > complicates review and increases the likeliness of bugs. Here's the stat. Note that removing the functionality as discussed would remove all of

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Michael Paquier
On Fri, Nov 25, 2016 at 3:31 AM, Pavel Stehule wrote: > 2016-11-24 18:51 GMT+01:00 Tom Lane : >> contrib/xml2 has always relied on libxslt for xpath functionality. >> Can we do that here instead of writing, debugging, and documenting >> a pile of new

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Pavel Stehule
2016-11-24 18:51 GMT+01:00 Tom Lane : > Alvaro Herrera writes: > > Pavel Stehule wrote: > >> 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : > >>> Oh my, I just noticed we have a new xpath preprocessor in this patch > >>> too.

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Tom Lane
Alvaro Herrera writes: > Pavel Stehule wrote: >> 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : >>> Oh my, I just noticed we have a new xpath preprocessor in this patch >>> too. Where did this code come from -- did you write it all from >>>

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Alvaro Herrera
Pavel Stehule wrote: > can me send your last work? Sure, it's in the archives -- https://www.postgresql.org/message-id/20161123233130.oqf7jl6czehy5fiw@alvherre.pgsql -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training &

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Pavel Stehule
2016-11-24 18:29 GMT+01:00 Alvaro Herrera : > Pavel Stehule wrote: > > Hi > > > > 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : > > > > > Oh my, I just noticed we have a new xpath preprocessor in this patch > > > too. Where did this code come from

Re: [HACKERS] patch: function xmltable

2016-11-24 Thread Alvaro Herrera
Pavel Stehule wrote: > Hi > > 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : > > > Oh my, I just noticed we have a new xpath preprocessor in this patch > > too. Where did this code come from -- did you write it all from > > scratch? > > I wrote it from scratch - libxml2

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Pavel Stehule
2016-11-24 5:52 GMT+01:00 Pavel Stehule : > Hi > > 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : > >> Oh my, I just noticed we have a new xpath preprocessor in this patch >> too. Where did this code come from -- did you write it all from >>

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Pavel Stehule
2016-11-24 0:29 GMT+01:00 Alvaro Herrera : > Here's another version. Not there yet: need to move back the function > to create the tupdesc, as discussed. Not clear what's the best place, > however. I modified the grammar a bit (added the missing comma, removed > PATH

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Pavel Stehule
Hi 2016-11-24 0:13 GMT+01:00 Alvaro Herrera : > Oh my, I just noticed we have a new xpath preprocessor in this patch > too. Where did this code come from -- did you write it all from > scratch? > I wrote it from scratch - libxml2 has not any API for iteration over

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Alvaro Herrera
Alvaro Herrera wrote: > If you use "PATH '/'" for a column, you get the text for all the entries > in the whole XML, rather than the text for the particular row being > processed. Isn't that rather weird, or to put it differently, completely > wrong? I didn't find a way to obtain the whole XML

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Andrew Dunstan
On 11/23/2016 06:31 PM, Alvaro Herrera wrote: Sorry, here's the patch. Power loss distracted me here. By the way, the pgindent you did is slightly confused because you failed to add the new struct types you define to typedefs.list. Tips on how to use pgindent for developers:

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Alvaro Herrera
Here's another version. Not there yet: need to move back the function to create the tupdesc, as discussed. Not clear what's the best place, however. I modified the grammar a bit (added the missing comma, removed PATH as an unreserved keyword and just used IDENT, removed the "Opt" version for

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Alvaro Herrera
Oh my, I just noticed we have a new xpath preprocessor in this patch too. Where did this code come from -- did you write it all from scratch? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Alvaro Herrera
Alvaro Herrera wrote: > I'm still wondering how this works. I'll continue to explore the patch > in order to figure out. Ah, so it's already parsed as a "range function". That part looks good to me. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7

Re: [HACKERS] patch: function xmltable

2016-11-23 Thread Alvaro Herrera
I tried to see if a following RTE was able to "see" the entries created by XMLTABLE, and sure it can: SELECT X.*, generate_series FROM emp, XMLTABLE ('//depts/dept/employee' passing doc COLUMNS empIDINTEGER PATH '@id', firstnamevarchar(25) PATH 'name/first',

Re: [HACKERS] patch: function xmltable

2016-11-22 Thread Pavel Stehule
2016-11-22 21:47 GMT+01:00 Alvaro Herrera : > I found the whole TableExprGetTupleDesc() function a bit odd in > nodeFuncs.c, so I renamed it to ExecTypeFromTableExpr() and moved it to > execTuples.c -- but only because that's where ExecTypeFromTL and others > already

Re: [HACKERS] patch: function xmltable

2016-11-22 Thread Alvaro Herrera
I found the whole TableExprGetTupleDesc() function a bit odd in nodeFuncs.c, so I renamed it to ExecTypeFromTableExpr() and moved it to execTuples.c -- but only because that's where ExecTypeFromTL and others already live. I would have liked to move it to tupdesc.c instead, but it requires

Re: [HACKERS] patch: function xmltable

2016-11-21 Thread Pavel Stehule
2016-11-21 21:16 GMT+01:00 Tom Lane : > Alvaro Herrera writes: > > Something I just noticed is that transformTableExpr takes a TableExpr > > node and returns another TableExpr node. That's unlike what we do in > > other places, where the node

Re: [HACKERS] patch: function xmltable

2016-11-21 Thread Tom Lane
Alvaro Herrera writes: > Something I just noticed is that transformTableExpr takes a TableExpr > node and returns another TableExpr node. That's unlike what we do in > other places, where the node returned is of a different type than the > input node. I'm not real

Re: [HACKERS] patch: function xmltable

2016-11-21 Thread Alvaro Herrera
Something I just noticed is that transformTableExpr takes a TableExpr node and returns another TableExpr node. That's unlike what we do in other places, where the node returned is of a different type than the input node. I'm not real clear what happens if you try to re-transform a node that was

Re: [HACKERS] patch: function xmltable

2016-11-18 Thread Pavel Stehule
2016-11-19 5:19 GMT+01:00 Pavel Stehule : > > > 2016-11-19 0:42 GMT+01:00 Alvaro Herrera : > >> The SQL standard seems to require a comma after the XMLNAMESPACES >> clause: >> >> ::= >> XMLTABLE >> [ ] >> >> [ ] >> COLUMNS

Re: [HACKERS] patch: function xmltable

2016-11-18 Thread Pavel Stehule
2016-11-19 0:42 GMT+01:00 Alvaro Herrera : > The SQL standard seems to require a comma after the XMLNAMESPACES > clause: > > ::= > XMLTABLE > [ ] > > [ ] > COLUMNS > > I don't understand the reason for that, but I have added it: > >

Re: [HACKERS] patch: function xmltable

2016-11-18 Thread Alvaro Herrera
The SQL standard seems to require a comma after the XMLNAMESPACES clause: ::= XMLTABLE [ ] [ ] COLUMNS I don't understand the reason for that, but I have added it: | XMLTABLE '(' XMLNAMESPACES '(' XmlNamespaceList ')' ',' c_expr xmlexists_argument

Re: [HACKERS] patch: function xmltable

2016-11-18 Thread Alvaro Herrera
Pavel Stehule wrote: > 2016-11-17 19:22 GMT+01:00 Alvaro Herrera : > > > Next, looking at struct TableExprBuilder I noticed that the comments are > > already obsolete, as they talk about function params that do not exist > > (missing_columns) and they fail to mention

Re: [HACKERS] patch: function xmltable

2016-11-18 Thread Pavel Stehule
Hi 2016-11-17 19:22 GMT+01:00 Alvaro Herrera : > I've been going over this patch. I think it'd be better to restructure > the before adding the docs for this new function; I already > split it out, so don't do anything about this. > > Next, looking at struct

Re: [HACKERS] patch: function xmltable

2016-11-17 Thread Alvaro Herrera
I've been going over this patch. I think it'd be better to restructure the before adding the docs for this new function; I already split it out, so don't do anything about this. Next, looking at struct TableExprBuilder I noticed that the comments are already obsolete, as they talk about

Re: [HACKERS] patch: function xmltable

2016-10-13 Thread Pavel Stehule
Hi new update - only doc + + Only XPath query language is supported. PostgreSQL doesn't support XQuery + language. Then the syntax of xmltable doesn't + allow to use XQuery related functionality - the name of xml expression + (clause AS) is not allowed, and only one xml

Re: [HACKERS] patch: function xmltable

2016-10-02 Thread Michael Paquier
On Tue, Sep 27, 2016 at 2:29 PM, Pavel Stehule wrote: > 2016-09-27 5:53 GMT+02:00 Craig Ringer : >> >> [...] >> ... so please delete that text. I thought I'd tested it but the state >> of my tests dir says I just got distracted by another task at

Re: [HACKERS] patch: function xmltable

2016-09-26 Thread Pavel Stehule
2016-09-27 5:53 GMT+02:00 Craig Ringer : > On 24 September 2016 at 14:01, Pavel Stehule > wrote: > > >> Did some docs copy-editing and integrated some examples. Explained how > >> nested elements work, that multiple top level elements is an error,

Re: [HACKERS] patch: function xmltable

2016-09-26 Thread Craig Ringer
On 24 September 2016 at 14:01, Pavel Stehule wrote: >> Did some docs copy-editing and integrated some examples. Explained how >> nested elements work, that multiple top level elements is an error, >> etc. Explained the time-of-evaluation stuff. Pointed out that you can

Re: [HACKERS] patch: function xmltable

2016-09-26 Thread Pavel Stehule
2016-09-27 3:34 GMT+02:00 Craig Ringer : > On 24 September 2016 at 14:01, Pavel Stehule > wrote: > > >> Did some docs copy-editing and integrated some examples. Explained how > >> nested elements work, that multiple top level elements is an error,

Re: [HACKERS] patch: function xmltable

2016-09-26 Thread Craig Ringer
On 24 September 2016 at 14:01, Pavel Stehule wrote: >> Did some docs copy-editing and integrated some examples. Explained how >> nested elements work, that multiple top level elements is an error, >> etc. Explained the time-of-evaluation stuff. Pointed out that you can

  1   2   >