Re: xpath processing brain dead

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nikolay Samokhvalov <samokhvalov(at)gmail(dot)com>
Subject: Re: xpath processing brain dead
Date: 2009-02-27 22:29:28
Message-ID: 49A86948.8080100@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>>> I'll do some tests to see what the cost of extra xml parsing might be.
>>>
>
>
>> The extra cost appears to be fairly negligible.
>>
>
> Uh, you didn't actually show a comparison of before and after?
> What it looks like to me is that this approach is free or nearly so for
> well-formed documents, but doubles the parsing cost for forests.
> Which is likely to annoy anyone who's really depending on the
> capability.
>

The difference is lost in the noise.

Without fix:

Time: 24.619 ms
Time: 24.245 ms
Time: 25.179 ms

With fix:

Time: 24.084 ms
Time: 21.980 ms
Time: 23.765 ms

The test is done on 10,000 short fragments each parsed 10 times (or 20
times with the fix).

I'll test again on some longer fragments since you don't seem convinced.

> Also,
>
>
>> ! if (*VARDATA(xpath_expr_text) == '/')
>>
>
> This is risking a core dump if the xpath expr is of zero length. You
> need something like
>
> if (xpath_len > 0 && *VARDATA(xpath_expr_text) == '/')
>

OK.

> It would also be a good idea if the allocation of string and xpath_expr
> had a comment about why it's allocating extra space (something like "see
> hacks below for use of this extra space" would be sufficient).
>

OK.

cheers

andrew

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2009-02-27 22:59:05 Re: Updates of SE-PostgreSQL 8.4devel patches (r1530)
Previous Message Tom Lane 2009-02-27 22:06:35 Re: Updates of SE-PostgreSQL 8.4devel patches (r1530)