On Mon, Dec 15, 2014 at 9:05 AM, Ali Akbar <the.apaan@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 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!
Sorry, typo here. What i mean isn't "invalid xml", but "incomplete xml". Hence the next call to xpath with the previous result as its input will become a problem because the namespace will not match.
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.
