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

2015-01-18 Thread Ali Akbar
2015-01-18 10:44 GMT+07:00 Peter Eisentraut pete...@gmx.net: On 1/7/15 8:51 PM, Ali Akbar wrote: So now +1 for back-patching this. Done. Was a bit of work to get it into all the old versions. Wow. Thanks. Btw, for bug-fix patches like this, should the patch creator (me) also create

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

2015-01-18 Thread David Fetter
On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: 2015-01-18 10:44 GMT+07:00 Peter Eisentraut pete...@gmx.net: On 1/7/15 8:51 PM, Ali Akbar wrote: So now +1 for back-patching this. Done. Was a bit of work to get it into all the old versions. Wow. Thanks. Btw, for

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

2015-01-18 Thread Michael Paquier
On Mon, Jan 19, 2015 at 2:38 AM, David Fetter da...@fetter.org wrote: On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: 2015-01-18 10:44 GMT+07:00 Peter Eisentraut pete...@gmx.net: Btw, for bug-fix patches like this, should the patch creator (me) also create patches for back branches?

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

2015-01-18 Thread Tom Lane
Michael Paquier michael.paqu...@gmail.com writes: On Mon, Jan 19, 2015 at 2:38 AM, David Fetter da...@fetter.org wrote: On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: 2015-01-18 10:44 GMT+07:00 Peter Eisentraut pete...@gmx.net: Btw, for bug-fix patches like this, should the patch

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

2015-01-17 Thread Peter Eisentraut
On 1/7/15 8:51 PM, Ali Akbar wrote: So now +1 for back-patching this. Done. Was a bit of work to get it into all the old versions. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

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

2015-01-06 Thread Peter Eisentraut
committed version 7 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

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

2015-01-06 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes: committed version 7 Isn't that a back-patchable bug fix? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

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

2014-12-14 Thread Ali Akbar
I ran a test using postgres-US.fo built in the PostgreSQL source tree, which is 38 MB, and ran select unnest(xpath('//fo:bookmark-title', b, array[array['fo', 'http://www.w3.org/1999/XSL/Format']])) from data; (Table contains one row only.) The timings were basically indistinguishable

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

2014-12-14 Thread Ali Akbar
2014-12-14 22:18 GMT+07:00 Ali Akbar the.ap...@gmail.com: I ran a test using postgres-US.fo built in the PostgreSQL source tree, which is 38 MB, and ran select unnest(xpath('//fo:bookmark-title', b, array[array['fo', 'http://www.w3.org/1999/XSL/Format']])) from data; (Table contains one

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

2014-12-14 Thread Michael Paquier
On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar the.ap...@gmail.com wrote: Peter, while reviewing the better performing patch myself, now i think the patch needs more work to be committed. The structuring of the method will be confusing in the long term. I think I'll restructure the patch in the

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

2014-12-14 Thread Ali Akbar
2014-12-15 10:19 GMT+07:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar the.ap...@gmail.com wrote: Peter, while reviewing the better performing patch myself, now i think the patch needs more work to be committed. The structuring of the method will

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

2014-12-14 Thread Ali Akbar
2014-12-15 11:02 GMT+07:00 Ali Akbar the.ap...@gmail.com: 2014-12-15 10:19 GMT+07:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar the.ap...@gmail.com wrote: Peter, while reviewing the better performing patch myself, now i think the patch needs more

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

2014-12-14 Thread Michael Paquier
On Mon, Dec 15, 2014 at 1:02 PM, Ali Akbar the.ap...@gmail.com wrote: 2014-12-15 10:19 GMT+07:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar the.ap...@gmail.com wrote: Peter, while reviewing the better performing patch myself, now i think the

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

2014-12-14 Thread Ali Akbar
2014-12-15 11:06 GMT+07:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 15, 2014 at 1:02 PM, Ali Akbar the.ap...@gmail.com wrote: 2014-12-15 10:19 GMT+07:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar the.ap...@gmail.com wrote: Peter,

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

2014-12-11 Thread Peter Eisentraut
On 11/5/14 9:50 AM, Ali Akbar wrote: I noticed somewhat big performance regression if the xml is large (i use PRODML Product Volume sample from energistics.org http://energistics.org): * Without patch (tested 3 times): select unnest(xpath('//a:flow', x,

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

2014-11-05 Thread Ali Akbar
2014-11-04 22:16 GMT+07:00 Peter Eisentraut pete...@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

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

2014-11-04 Thread Peter Eisentraut
On 7/11/14 4:36 AM, Ali Akbar wrote: But i found some bug in libxml2's code, because we call xmlCopyNode with 1 as the second parameter, internally xmlCopyNode will call xmlStaticCopyNode recursively via xmlStaticCopyNodeList. And xmlStaticCopyNodeList doesn't check the return of

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

2014-11-04 Thread Peter Eisentraut
On 7/8/14 6:03 AM, Ali Akbar wrote: If we put 1 as the parameter, the resulting Node will link to the existing children. In this case, the namespace problem for the children might be still exists. If any of the children uses different namespace(s) than the parent, the namespace definition will

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

2014-11-04 Thread Peter Eisentraut
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

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

2014-10-06 Thread Ali Akbar
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

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

2014-08-29 Thread Ali Akbar
Greetings, Because of the memory bug in xmlCopyNode, this is a new patch with different method. In this patch, instead of using xmlCopyNode to bring the namespace back, we added the required namespaces to the node before dumping the node to string, and cleaning it up afterwards (because the same

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

2014-07-11 Thread Ali Akbar
Greetings, Attached modified patch to handle xmlCopyNode returning NULL. The patch is larger because xmlerrcxt must be passed to xml_xmlnodetoxmltype (xmlerrcxt is needed for calling xml_ereport). From libxml2 source, it turns out that the other cases that xmlCopyNode will return NULL will not

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

2014-07-08 Thread Ali Akbar
I don't know enough about XML/XPATH to know if this is a good idea or not, Actually currently because of the namespace problem, xpath() returns wrong result (even worse: invalid xml!). So this patch corrects the behavior of xpath() to the correct one. but I did go read the documentation of

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

2014-07-07 Thread Tom Lane
Abhijit Menon-Sen a...@2ndquadrant.com writes: I can confirm that it works fine. I have attached here a very slightly tweaked version of the patch (removed trailing whitespace, and changed some comment text). I'm marking this ready for committer. I don't know enough about XML/XPATH to know

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

2014-06-22 Thread Ali Akbar
2014-06-16 22:52 GMT+07:00 Abhijit Menon-Sen a...@2ndquadrant.com: Thanks for the patch, and welcome to Postgres development. I can confirm that it works fine. I have attached here a very slightly tweaked version of the patch (removed trailing whitespace, and changed some comment text).