Re: KNN-GiST with recheck

On 05/14/2015 01:43 PM, Alexander Korotkov wrote:
> On Wed, May 13, 2015 at 10:17 PM, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
> wrote:
>
>> One quick comment:
>>>
>>> It would be good to avoid the extra comparisons of the distances, when
>>> the index doesn't return any lossy items. As the patch stands, it adds one
>>> extra copyDistances() call and a cmp_distances() call for each tuple (in a
>>> knn-search), even if there are no lossy tuples.
>>
>>
>> I will fix it until Friday.
>>
>
> Attached patch is rebased against current master. Extra extra
> copyDistances() call and a cmp_distances() call for each tuple are avoided
> in the case of no lossy tuples.

Thanks! I spent some time cleaning this up:

* fixed a memory leak

* fixed a silly bug in rechecking multi-column scans

* I restructured the changes to IndexNext. I actually created a whole
separate copy of IndexNext, called IndexNextWithReorder, that is used
when there are ORDER BY expressions that might need to be rechecked.
There is now some duplicated code between them, but I think they are
both easier to understand this way. The IndexNext function is now as
simple as before, and the IndexNextWithReorder doesn't need so many
if()-checks on whether the reorder queue exists at all.

* I renamed Distance to OrderByValues in the executor parts. We call the
"ORDER BY x <-> y" construct an ORDER BY expression, so let's continue
using that terminology.

I think this is now ready for committing, but I'm pretty tired now so
I'll read through this one more time in the morning, so that I won't
wake up to a red buildfarm.

- Heikki