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

2014-12-15 10:19 GMT+07:00 Michael Paquier <michael(dot)paquier(at)gmail(dot)com>:
>
> On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar <the(dot)apaan(at)gmail(dot)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
> next
> > commitfest.
> > So i propose to break the patch:
> > 1. We apply the current patch which uses xmlNodeCopy, so that the
> > long-standing bug will be fixed in postgres.
> > 2. I'll work with the performance enhancement in the next commitfest.
> >
> > Maybe for (2), the current better-performing patch can be viewed as PoC
> of
> > the expected performance.
>
> Ali, are you currently working on that? Would you mind re-creating new
> entries in the commit fest app for the new set of patches that you are
> planning to do?
> For now I am switching this patch as returned with feedback.
> Thanks,
>

What i mean, the last patch (v7 patch) as it is is enough to fix the bug
(nested xpath namespace problem). I think the performance regression is
still acceptable (in my case it's ~20%), because the bug is severe.
Currently, xpath can return invalid xml because the namespace isn't
included in the output!

What i'll be working is the v4 patch, because it turns out the v4 patch has
better performance (~10%, but Peter's test shows it isn't the case). But,
the problem is the v4 patch is organized wrongly, and hacks around the
libxml's xml node structure (duplicating the namespace on the existing
structure). I'll work on that, but it will affects the performance benefit.

So what i propose is, we close the longstanding bug in this comitfest with
the v7 patch. I'll work on improving the performance, without compromising
good code structure. If the result is good, i'll submit the patch.

Thanks

Regards,
--
Ali Akbar