Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-14 Thread Radosław Smogura
On Thu, 14 Jul 2011 15:15:33 +0300, Peter Eisentraut wrote: On sön, 2011-07-10 at 11:40 -0700, Josh Berkus wrote: Hackers, B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this content

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-14 Thread Peter Eisentraut
On ons, 2011-07-13 at 11:58 +0200, Nicolas Barbier wrote: 2011/6/29, Florian Pflug f...@phlo.org: Secondly, there is little point in having an type XML if we don't actually ensure that values of that type can only contain well-formed XML. +1. The fact that XPATH() must return a type

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-14 Thread Peter Eisentraut
On sön, 2011-07-10 at 11:40 -0700, Josh Berkus wrote: Hackers, B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this content is actually . I don't buy that. The check for the

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-13 Thread Radosław Smogura
On Tue, 12 Jul 2011 11:11:40 -0700, Josh Berkus wrote: Radoslaw, For me this discussion is over. I putted my objections and suggestions. Full review is available in archives, and why to not escape is putted in review of your 2nd patch, about scalar values. Did you install and test the

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-13 Thread Nicolas Barbier
2011/6/29, Florian Pflug f...@phlo.org: Secondly, there is little point in having an type XML if we don't actually ensure that values of that type can only contain well-formed XML. +1. The fact that XPATH() must return a type that cannot depend on the given expression (even if it is a

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Radosław Smogura
On Sun, 10 Jul 2011 17:06:22 -0500, Robert Haas wrote: On Jul 10, 2011, at 1:40 PM, Josh Berkus j...@agliodbs.com wrote: Hackers, B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Florian Pflug
On Jul12, 2011, at 11:00 , Radosław Smogura wrote: On Sun, 10 Jul 2011 17:06:22 -0500, Robert Haas wrote: Unless I am missing something, Florian is clearly correct here. For me not, because this should be fixed internally by making xml type sefe Huh??. Making the xml type safe is *exactly*

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Radosław Smogura
On Tue, 12 Jul 2011 11:45:59 +0200, Florian Pflug wrote: On Jul12, 2011, at 11:00 , Radosław Smogura wrote: On Sun, 10 Jul 2011 17:06:22 -0500, Robert Haas wrote: Unless I am missing something, Florian is clearly correct here. For me not, because this should be fixed internally by making xml

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Florian Pflug
On Jul12, 2011, at 12:57 , Radosław Smogura wrote: On Tue, 12 Jul 2011 11:45:59 +0200, Florian Pflug wrote: On Jul12, 2011, at 11:00 , Radosław Smogura wrote: On Sun, 10 Jul 2011 17:06:22 -0500, Robert Haas wrote: Unless I am missing something, Florian is clearly correct here. For me not,

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Josh Berkus
Radoslaw, For me this discussion is over. I putted my objections and suggestions. Full review is available in archives, and why to not escape is putted in review of your 2nd patch, about scalar values. Did you install and test the functionality of the patch? I can't tell from your review

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-12 Thread Josh Berkus
Florian, Radoslaw, Please both of you calm down. Florian is trying to improve our XML type. Radoslaw is trying to help out by reviewing it. It's not a benefit to anyone for you two to get into an argument about who said what ... especially if the argument is based on (as far as I can see) not

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-10 Thread Josh Berkus
Hackers, B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this content is actually . I don't buy that. The check for the node type is there because two different libxml functions are

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-07-10 Thread Robert Haas
On Jul 10, 2011, at 1:40 PM, Josh Berkus j...@agliodbs.com wrote: Hackers, B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this content is actually . I don't buy that. The check

[HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-06-29 Thread Radosław Smogura
Review of patch https://commitfest.postgresql.org/action/patch_view?id=580 === Patch description === SUMMARY: When text() based XPATH expression is invoked output is not XML escaped DESCRIPTION: Submitter invokes following statement: SELECT (XPATH('/*/text()', 'rootlt;/root'))[1]. He expect

Re: [HACKERS] Patch Review: Bugfix for XPATH() if text or attribute nodes are selected

2011-06-29 Thread Florian Pflug
On Jun29, 2011, at 19:34 , Radosław Smogura wrote: B. 6. Current behaviour _is intended_ (there is if to check node type) and _natural_. In this particular case user ask for text content of some node, and this content is actually . I don't buy that. The check for the node type is there