Re: [REVIEW] Re: Fix xpath() to return namespace definitions

2014-11-04 22:16 GMT+07:00 Peter Eisentraut <peter_e(at)gmx(dot)net>:

> On 10/6/14 10:24 PM, Ali Akbar wrote:
> > While reviewing the patch myself, i spotted some formatting problems in
> > the code. Fixed in this v5 patch.
> >
> > Also, this patch uses context patch format (in first versions, because
> > of the small modification, context patch format obfucates the changes.
> > After reimplementation this isn't the case anymore)
>
> I think the problem this patch is addressing is real, and your approach
> is sound, but I'd ask you to go back to the xmlCopyNode() version, and
> try to add a test case for why the second argument = 1 is necessary. I
> don't see any other problems.
>

OK. Because upstream code is fixed in current version, i'll revert to the
previous version. Test case added to regression test. With =1 argument, the
result is correct:
<local:piece xmlns:local=\"http://127.0.0.1\" xmlns=\"http://127.0.0.2\"
id=\"1\">
<internal>number one</internal>
<internal2/>
</local:piece>

without the argument, the result is not correct, all children will be lost.
Because of that, the other regression test will fail too because the
children is not copied:
*** 584,593 ****

-- Text XPath expressions evaluation
SELECT xpath('/value', data) FROM xmltest;
! xpath
! ----------------------
! {<value>one</value>}
! {<value>two</value>}
(2 rows)

SELECT xpath(NULL, NULL) IS NULL FROM xmltest;
--- 584,593 ----

-- Text XPath expressions evaluation
SELECT xpath('/value', data) FROM xmltest;
! xpath
! ------------
! {<value/>}
! {<value/>}
(2 rows)

SELECT xpath(NULL, NULL) IS NULL FROM xmltest;
***************
... <cut>

updated patch attached.

Regards,
--
Ali Akbar