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

Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> writes:
> For master and 9.4, I'm thinking of applying the attached. It makes it
> clear that startScan() is not used to re-start a scan with existing scan
> keys, but is always called on a newly initialized scan keys.

Looks reasonable to me, but should ginFreeScanKeys() null out the pointers
after freeing them, to be sure we find any incorrect accesses? It might
not be worth the trouble; but if you have any doubts at all about the
order of operations this seems like a good safety feature.

Also, in the department of nitpicks, I'd do this:

{
IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
+ GinScanOpaque so = (GinScanOpaque) scan->opaque;
TIDBitmap *tbm = (TIDBitmap *) PG_GETARG_POINTER(1);
int64 ntids;

more like this:

{
IndexScanDesc scan = (IndexScanDesc) PG_GETARG_POINTER(0);
TIDBitmap *tbm = (TIDBitmap *) PG_GETARG_POINTER(1);
+ GinScanOpaque so = (GinScanOpaque) scan->opaque;
int64 ntids;

I think of the PG_GETARG calls as being an ugly stepchild of a proper
function header declaration, and as such, they should come first unless
there is an unavoidable reason not to.

regards, tom lane