Re: BUG #12694: crash if the number of result rows is lower than gin_fuzzy_search_limit

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