|From:||Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>|
|To:||Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>|
|Cc:||Craig Ringer <craig(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: patch: function xmltable|
|Views:||Raw Message | Whole Thread | Download mbox|
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 column options), and reworked the comments in the transform
phase (I tweaked the code here and there mostly to move things to nicer
places, but it's pretty much the same code).
In the new xpath_parser.c file I think we should tidy things up a bit.
First, it needs more commentary on what the entry function actually
does, in detail. Also, IMO that function should be at the top of the
file, not at the bottom, followed by all its helpers. I would like some
more clarity on the provenance of all this code, just to assess the
probability of bugs; mostly as it's completely undocumented.
I don't like the docs either. I think we should have a complete
reference to the syntax, followed by examples, rather than letting the
examples drive the whole thing. I fixed the synopsis so that it's not
one very long line.
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 row when you have
the COLUMNS option (which is what I was hoping for with the "PATH '/'").
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
|Next Message||Alvaro Herrera||2016-11-23 23:31:30||Re: patch: function xmltable|
|Previous Message||Alvaro Herrera||2016-11-23 23:13:30||Re: patch: function xmltable|