[HACKERS] Review of patch Bugfix for XPATH() if expression returns a scalar value

[Radosław Smogura]

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="&amp;lt;n"/>
 
        It's clearly visible that value from attribute is "&lt;n", not "<". 
Every 
parser will read this as "&lt;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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers