Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit - Mailing list pgsql-bugs

From Heikki Linnakangas
Subject Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit
Date
Msg-id 54CA57B9.3070602@vmware.com
Whole thread Raw
In response to Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
On 01/29/2015 05:26 PM, Tom Lane wrote:
> Heikki Linnakangas <hlinnakangas@vmware.com> writes:
>> The fix is simple: make sure that startScanKey() is always called, by
>> getting rid of the early return above. Attached. I'll apply this later
>> today or tomorrow unless someone sees a problem with this.
>
> Another, even simpler fix would be to just move the startScanKey()
> call loop to before the "if (GinFuzzySearchLimit > 0)" block.
> Is there a particular reason why it's a good idea to do things in
> the current order?  It almost looks like a patch application error
> as it stands.

The code in startScanKey() uses the predictNumberResult estimates, which
the loop modifies, so it would change how that behaves. I'm not sure if
it would actually be better that way though; it's not clear to me how
the fuzzy search limit should interact with the fast scan code. It won't
affect correctness either way.

The for()-loop sets the so->entries[i]-reduceResult fields, so it makes
sense to do it right after the startScanEntry() calls. Otherwise the
logic would be "1. do some initialization on entries, 2. initialize
keys, 3. do more initialization on entries", which would be a bit strange.

<looks at the loop a bit more>

Hmm, isn't the if-check in the second loop unnecessary? The first loop
checks that so->entries[i]->predictNumberResult > so->totalentries *
GinFuzzySearchLimit for all entries. If there are any where that doesn't
hold, it returns (which is a bad idea). There's no need to check for the
same condition again in the second loop.

- Heikki

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit
Next
From: Tom Lane
Date:
Subject: Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit