Re: bug of pg_trgm?

On Thu, Aug 9, 2012 at 2:28 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> When I used pg_trgm, I encountered the problem that the search result of
>> SeqScan was the different from that of BitmapScan even if the search
>> keyword was the same. Is this a bug?
>
> Surely.
>
>> The cause is ISTM that pg_trgm wrongly ignores the heading wildcard
>> character (i.e., %) when an escape (i.e., \\) follows the wildcard character.
>> Attached patch fixes this.
>
> This patch doesn't seem quite right to me, though. I agree that given
> '%\x...', we should exit the loop with in_wildcard_meta still true.
> However, if we have say '%\+...', the loop will continue, and now we
> must reset in_wildcard_meta, no? The next character is not adjacent to
> a meta. So I think in the "if (in_escape)" block, *both* assignments
> should be moved after the iswordchr check. Is there something I'm
> missing?

No. You're right. We must reset in_wildcard_meta and must ignore %\+
in '%\+...'.

> Also, shouldn't we make a similar change in the second loop? I guess
> it's not strictly necessary given that that loop will exit as soon as
> it sets in_wildcard_meta, but if you want to depend on that then the
> reset in the second "if (in_escape)" block is altogether useless. It
> seems better to keep the logic of the two loops as similar as possible.

Yes. There is another useless reset of in_wildcard_meta in the second
loop. We should also remove that?

> I'm also inclined to think that we should remove *both* flag resets
> before the second loop. The logic here is that we are reprocessing
> the same character seen in the last iteration of the first loop,
> right? So the flag state ought to remain the same.

No. ISTM that in_wildcard_meta must be reset before the second loop.
Because the meaning of that flag in the first loop is different from that in
the second loop. The former and the latter indicate whether the search
string has *heading* and *tailing* wildcard character, respectively. No?

Regards,

--
Fujii Masao