Thread

1. xpath improvement V2

   Arie Bikker <arie@abikker.nl> — 2010-02-12T21:32:08Z

   Hi all,
   
   I've combined the review suggestions of Jan Urbański, Scott Bailey, and
   others.
   This was a lot harder, then I had foreseen; and I took my time to do it
   the right way (hope you agree!).
   
   BTW. really appreciate your efforts, so far, to enlighten me on nuub 
   errors/mistakes in the previous version.
   
   Additional improvement (hence the V2):
   two extra functions: xpath_value_text and xpath_value_strict
   Both are quite general: xpath_value_text maps everything to text, except 
   nodeset. xpath_value_strict has to be told exactly what to expect.
   xpath_value_text(text xpath, xml doc [, namespace]) returns text
   xpath_value_strict(anyelement typexample, text xpath, xml doc [, 
   namespace]) returns anyelement
     See the doc for further explanation.
   I chose this approach, as opposed to xpath_value_string/number/boolean 
   for a couple of reasons:
   - We want postgresql functions, with postgresql types. The functions 
   should fit postgresql usage and hide libxml parlance.
   - Functions in pg_catalog should be destined for broad use, not just 
   satisfy the adhoc desire of the implementer.
   - Function synopsis should be adequate to withstand libxml extension or 
   improvements (libxml3?)
   - Construction of XPath expressions for value retrieval is not trivial. 
   A precise calling syntax, hopefully, focuses the user to select the 
   right expressions.
   - Loose implementation with "autocasting" opens the door for unwanted 
   injection possibilities.
   
   Lastly, when in need of a xpath_value_string (et al), it can easily be 
   added by the user through:
   CREATE FUNCTION xpath_value_string(text, xml) RETURNS text AS $$
   SELECT xpath_value_strict('a'::text,$1,$2);
   $$ LANGUAGE SQL;
   
   Points fixed:
   - source code indentation. (manually though, can't get pgindent to work
   properly)
   - Doc entries with some examples
   - test/regress entries
   - Detailed directions from Jan Urbanski on ereport and PG_TRY/CATCH.
   
   Here's a shortlist of subjects you should definetely review (aka I'me 
   not certain these are up to standards)
   - calling style in xpath_value_strict with typexample. I really like it,
   but I haven't seen such an approach elsewhere in the API.
   - tone and style in doc. I'me not native English speaker and had to 
   learn DocBook along the way.
   - insertion in catalog/pg_proc.h. Just took some free oid numbers, and 
   fiddled the other parameters.
   - Is ERRCODE_DATA_EXCEPTION (in xml.c) acceptable as error code for a 
   type mismatch? Could not find anything better.
   
   kind regards, Arie Bikker
   
   

2. Re: xpath improvement V2

   Arie Bikker <arie@abikker.nl> — 2010-02-12T21:49:45Z

   And here is the patch.
   

3. Re: xpath improvement V2 with xml.c

   Arie Bikker <arie@abikker.nl> — 2010-02-12T22:06:48Z

   And a patch for the code implementing this
   
   

4. Re: xpath improvement V2

   Jan Urbański <wulczer@wulczer.org> — 2010-02-16T09:05:47Z

   Arie Bikker wrote:
   > Hi all,
   > 
   > I've combined the review suggestions of Jan Urbański, Scott Bailey, and
   > others.
   > This was a lot harder, then I had foreseen; and I took my time to do it
   > the right way (hope you agree!).
   
   Hi,
   
   I see the patch has been marked as "Returned with Feedback" on the 6th
   of February, I assume on grounds of prolonged silence about it. I
   confess it was partly my fault, because soon after posting the review I
   suddenly had to focus on other things.
   
   I won't be able to review the new version in the next few days, and this
   commitfest is closing anyway... However I would hate to see that patch
   just disappear, as I think it's useful and you obviously invested some
   work in it.
   
   At this stage I would suggest moving it to the first 9.1 commitfest,
   since it's a nice feature, but not one we should burden the committers
   with this late in the development cycle.
   
   Arie, care to add that last version of the patch to the 2010-Next
   commitfest?
   
   Cheers,
   Jan
   
   
   

5. Re: xpath improvement V2

   artacus@comcast.net — 2010-02-17T01:25:28Z

   Jan Urbański wrote:
   > Arie Bikker wrote:
   >> Hi all,
   >>
   >> I've combined the review suggestions of Jan Urbański, Scott Bailey, and
   >> others.
   >> This was a lot harder, then I had foreseen; and I took my time to do it
   >> the right way (hope you agree!).
   > 
   > Hi,
   > 
   > I see the patch has been marked as "Returned with Feedback" on the 6th
   > of February, I assume on grounds of prolonged silence about it. I
   > confess it was partly my fault, because soon after posting the review I
   > suddenly had to focus on other things.
   > 
   > I won't be able to review the new version in the next few days, and this
   > commitfest is closing anyway... However I would hate to see that patch
   > just disappear, as I think it's useful and you obviously invested some
   > work in it.
   > 
   > At this stage I would suggest moving it to the first 9.1 commitfest,
   > since it's a nice feature, but not one we should burden the committers
   > with this late in the development cycle.
   > 
   > Arie, care to add that last version of the patch to the 2010-Next
   > commitfest?
   > 
   > Cheers,
   > Jan
   
   I would agree w/ Jan about just adding to 9.1 except that it fixes 
   several known bugs.
   
   Scott