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

Michael Paquier writes: > On Mon, Jan 19, 2015 at 2:38 AM, David Fetter wrote: >> On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: >>> 2015-01-18 10:44 GMT+07:00 Peter Eisentraut : Btw, for bug-fix patches like this, should the patch creator (me) also create patches for back b

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

On Mon, Jan 19, 2015 at 2:38 AM, David Fetter wrote: > On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: >> 2015-01-18 10:44 GMT+07:00 Peter Eisentraut : >> Btw, for bug-fix patches like this, should the patch creator (me) also >> create patches for back branches? > > As I understand it,

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

On Sun, Jan 18, 2015 at 06:05:05PM +0700, Ali Akbar wrote: > 2015-01-18 10:44 GMT+07:00 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. > > > > Wow. Thanks. > > Btw, for bug

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

2015-01-18 10:44 GMT+07:00 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. > Wow. Thanks. Btw, for bug-fix patches like this, should the patch creator (me) also create patches for bac

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

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

> > Peter Eisentraut writes: > > committed version 7 > Thanks! 2015-01-07 13:28 GMT+07:00 Tom Lane : > Isn't that a back-patchable bug fix? > Upthread, i noted: > For back versions, i think because this patch changes xpath() behavior, we > will only apply this to future versions. The old behav

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

Peter Eisentraut 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: http://www.postgresql.org/mailpref/pgsql-hackers

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

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

2014-12-15 11:06 GMT+07:00 Michael Paquier : > > On Mon, Dec 15, 2014 at 1:02 PM, Ali Akbar wrote: > > 2014-12-15 10:19 GMT+07:00 Michael Paquier : > >> > >> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote: > >> > Peter, while reviewing the better performing patch myself, now i think > >> > the

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

On Mon, Dec 15, 2014 at 1:02 PM, Ali Akbar wrote: > 2014-12-15 10:19 GMT+07:00 Michael Paquier : >> >> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote: >> > Peter, while reviewing the better performing patch myself, now i think >> > the >> > patch needs more work to be committed. The structuring

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

2014-12-15 11:02 GMT+07:00 Ali Akbar : > > 2014-12-15 10:19 GMT+07:00 Michael Paquier : >> >> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar wrote: >> > Peter, while reviewing the better performing patch myself, now i think >> the >> > patch needs more work to be committed. The structuring of the meth

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

2014-12-15 10:19 GMT+07:00 Michael Paquier : > > On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar 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 t

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

On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar 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 next > commitfest. >

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

2014-12-14 22:18 GMT+07:00 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 on

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

> > 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 indistingui

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

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 ): > * Without patch (tested 3 times): > select unnest(xpath('//a:flow', x, > ARRAY[['a','http://www.prodml.org

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

2014-11-05 21:50 GMT+07:00 Ali Akbar : > 2014-11-04 22:16 GMT+07:00 Peter Eisentraut : > >> 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

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

2014-11-04 22:16 GMT+07:00 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 s

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

2014-11-04 22:16 GMT+07:00 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 mo

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

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. >

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

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 w

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

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 xmlStaticC

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

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

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

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

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 h

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

> 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

Abhijit Menon-Sen 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 if this is a good

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

2014-06-16 22:52 GMT+07:00 Abhijit Menon-Sen : > 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). > > I'm marking th

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

At 2014-05-30 16:04:33 +0700, the.ap...@gmail.com wrote: > > While developing some XML processing queries, i stumbled on an old bug > mentioned in http://wiki.postgresql.org/wiki/Todo#XML: Fix Nested or > repeated xpath() that apparently mess up namespaces. Thanks for the patch, and welcome to Pos