2014-11-04 22:16 GMT+07:00 Peter Eisentraut <peter_e@gmx.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; ***************
