Message-ID: <55552F9D.4090603@iki.fi>
Date: Fri, 15 May 2015 02:28:29 +0300
From: Heikki Linnakangas <hlinnaka@iki.fi>
Reply-To: hlinnaka@iki.fi
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Icedove/31.6.0
MIME-Version: 1.0
To: Alexander Korotkov <aekorotkov@gmail.com>
CC: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: KNN-GiST with recheck
References: 
 <CAPpHfdvn5jxgh-rVMz7c2vXoYW_W7dGrJjQkVAtaCr=-ZMCADA@mail.gmail.com>
 <CAPpHfdvEpZAJrMBkNrCguG4fCK5QtZjARPti7LnL5chsSuD=Sg@mail.gmail.com>
 <20140926011800.GA8130@momjian.us>
 <CAPpHfdvDMS1_jqnzZ0FbGePWwCvStj4_h0xU2hYTnLcC6of3Rg@mail.gmail.com>
 <20140929021631.GG12447@momjian.us>
 <CAPpHfdvdL0dtoNBmbCsJ=W0bFE7A87hLOtoBN7Pe4hnXkQ9LxA@mail.gmail.com>
 <20141006093609.GA12899@hasegeli.com> <5490357C.8090800@vmware.com>
 <CAPpHfdtm=A3hrw4=mhNxN6U2Q9bx4BYDhW8gRLxgaZD3MAQKCA@mail.gmail.com>
 <CAPpHfdstkF4RW_1cYje=EZyTx5VRgUK_+L3c2ucBASdD156tVg@mail.gmail.com>
 <CAPpHfduGjVpTLVxwXmeMuFqR-ykH-ERdewF-KHxSQiRdw29-7g@mail.gmail.com>
 <54EC8D3E.2000908@2ndquadrant.com>
 <CAPpHfdsZVJnMMOuTUusJQmcR1QeRN4qc93-KE014_JHqYU717g@mail.gmail.com>
 <CAPpHfdv40pmc7N3CcpBDEcTAnLT6dWaN76LP5Q9xzTvvdwRMQg@mail.gmail.com>
 <5553A2FF.1030501@iki.fi>
 <CAPpHfdtGyAnZK6fRFAmSyR5+AbccgxTajLvGYzdvpUg70Wsg2A@mail.gmail.com>
 <CAPpHfdvDk4=VRPUGvLL0rwVTys247shUeckcURQ42Y3-2tidAg@mail.gmail.com>
In-Reply-To: 
 <CAPpHfdvDk4=VRPUGvLL0rwVTys247shUeckcURQ42Y3-2tidAg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 05/14/2015 01:43 PM, Alexander Korotkov wrote:
> On Wed, May 13, 2015 at 10:17 PM, Alexander Korotkov <aekorotkov@gmail.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers