Re: [REVIEW] Re: Fix xpath() to return namespace definitions - Mailing list pgsql-hackers

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.