Re: Allow multi-byte characters as escape in SIMILAR TO and SUBSTRING

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Allow multi-byte characters as escape in SIMILAR TO and SUBSTRING
Date: 2014-08-27 05:13:46
Message-ID: 1409116426.2335.455.camel@jeff-desktop
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 2014-08-25 at 17:41 +0300, Heikki Linnakangas wrote:
> Actually, that gets optimized to a constant in the planner:

Oops, thank you (and Tom).

> your patch seems to be about 2x-3x as slow as unpatched master. So this
> needs some optimization. A couple of ideas:

I didn't see anywhere near that kind of regression. On unpatched master,
with your test case, I saw it stabilize to about 680ms. With
similar-escape-1, I saw about 775ms (15% regression). Are those at all
close to your numbers? Is there a chance you used an unoptimized build
for one of them, or left asserts enabled?

> 1. If the escape string is in fact a single-byte character, you can
> proceed with the loop just as it is today, without the pg_mblen calls.
>
> 2. Since pg_mblen() will always return an integer between 1-6, it would
> probably be faster to replace the memcpy() and memcmp() calls with
> simple for-loops iterating byte-by-byte.
>
> In very brief testing, with the 1. change above, the performance with
> this patch is back to what it's without the patch. See attached.

The particular patch has a mistake: the first branch is always taken
because pg_mblen() won't return 0. It's also fairly ugly to set mblen in
the test for the branch that doesn't use it.

Attached a patch implementing the same idea though: only use the
multibyte path if *both* the escape char and the current character from
the pattern are multibyte.

I also changed the comment to more clearly state the behavior upon which
we're relying. I hope what I said is accurate.

Regards,
Jeff Davis

Attachment Content-Type Size
similar-escape-3.patch text/x-patch 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2014-08-27 05:23:08 Re: Proposal for CSN based snapshots
Previous Message Michael Paquier 2014-08-27 05:07:39 Re: Missing comment block at the top of streamutil.h and receivelog.h