Re: bug of pg_trgm?

On Sat, Aug 11, 2012 at 8:15 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> 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?
>
> Oh, good point. Maybe it would be clearer to use two separate
> flag variables?

Agreed. Attached patch uses two separate flag variables.

On Sat, Aug 11, 2012 at 8:19 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
>> On Thu, Aug 9, 2012 at 3:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Probably a minimal fix for this could be made by backing up "endword"
>>> one byte before returning it if in_escape is true when the second
>>> loop exits. That would not scale up to preserving the state of
>>> in_wildcard_meta, but since the second loop never advances past a
>>> meta char, that's okay for the moment.
>
>> Or what about extending get_wildcard_part() so that it accepts the pointer
>> to in_escape as an argument? generate_wildcard_trgm() can know the last
>> value of in_escape and specify it the next call of get_wildcard_part(). Looks
>> very simple.
>
> Yeah, I had considered pushing the state variables out to the caller.
> If there were any prospect of wanting more state than just in_escape,
> I'd be for that --- but I don't see any reason to possibly need more,
> especially in view of your point that in_wildcard_meta isn't really
> a single flag with an interpretation that remains fixed throughout.
> I think it's probably better just to take care of the issue inside
> get_wildcard_part, and not complicate its API.

OK. Attached patch fixes the problem as you suggested, i.e., it backs up
"endword" if the second loop exits in an escape pair.

Regards,

--
Fujii Masao