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

From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: PG Hackers <pgsql-hackers(at)postgresql(dot)org>, Florian Pflug <fgp(at)phlo(dot)org>
Subject: Review of patch Bugfix for XPATH() if expression returns a scalar value
Date: 2011-06-29 17:57:03
Message-ID: 201106291957.04251.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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="&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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-06-29 18:11:34 Re: time-delayed standbys
Previous Message Simon Riggs 2011-06-29 17:50:22 Re: time-delayed standbys