Review of patch Bugfix for XPATH() if expression returns a scalar value - Mailing list pgsql-hackers

From Radosław Smogura
Subject Review of patch Bugfix for XPATH() if expression returns a scalar value
Date
Msg-id 201106291957.04251.rsmogura@softperience.eu
Whole thread Raw
Responses Re: Review of patch Bugfix for XPATH() if expression returns a scalar value
List pgsql-hackers
This is review of patch
https://commitfest.postgresql.org/action/patch_view?id=565
"Bugfix for XPATH() if expression returns a scalar value"

Patch applies cleanly, and compiles cleanly too, I didn't checked tests.
Form discussion about patch, and referenced thread in this patch 
http://archives.postgresql.org/pgsql-general/2010-07/msg00355.php, if I 
understand good such functionality is desired.
This patch, I think, gives good approach for dealing with scalar values, 
and, as well converting value to it's string representation is good too 
(according to current support for xml), with one exception detailed below.
In this patch submitter, similarly to 
https://commitfest.postgresql.org/action/patch_view?id=580, added 
functionality for XML-escaping of some kind of values (I think only string 
scalars are escaped), which is not-natural and may lead to double escaping in 
some situation, example query may be:

SELECT XMLELEMENT(name root, XMLATTRIBUTES(foo.namespace AS sth)) FROM (SELECT 
(XPATH('namespace-uri(/*)', x))[1] AS namespace FROM (VALUES (XMLELEMENT(name 
"root", XMLATTRIBUTES('<n' AS xmlns, '<v' AS value),'<t'))) v(x)) as foo;
      xmlelement        
-------------------------<root sth="&lt;n"/>It's clearly visible that value from attribute is "<n", not "<".
Every
 
parser will read this as "<n" which is not-natural and will require form 
consumer/developer to de-escape this on his side - roughly speaking this will 
be reported as serious bug.I didn't found good reasons why XML-escaping should be included, submitter 
wrote about inserting this to xml column and possibility of data damage, but 
didn't give an example. Such example is desired.

Conclusion
I vote +1 for this patch if escaping will be removed.

Regards,
Radoslaw Smogura


pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: time-delayed standbys
Next
From: Robert Haas
Date:
Subject: Re: time-delayed standbys