Re: [HACKERS] [PATCH] kNN for SP-GiST - Mailing list pgsql-hackers

From Nikita Glukhov
Subject Re: [HACKERS] [PATCH] kNN for SP-GiST
Date
Msg-id 43c859f9-cb0d-1bd2-ba9f-c5e5b61185cc@postgrespro.ru
Whole thread Raw
In response to Re: [HACKERS] [PATCH] kNN for SP-GiST  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: [HACKERS] [PATCH] kNN for SP-GiST  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers

Attached 3rd version of kNN for SP-GiST.


On 09.03.2017 16:52, Alexander Korotkov wrote:

Hi, Nikita!

I take a look to this patchset.  My first notes are following.

* 0003-Extract-index_store_orderby_distances-v02.patch

Function index_store_orderby_distances doesn't look general enough for its name.  GiST supports ordering only by float4 and float8 distances.  SP-GiST also goes that way.  But in the future, access methods could support distances of other types.
Thus, I suggest to rename function to  index_store_float8_orderby_distances().
This function was renamed.

* 0004-Add-kNN-support-to-SP-GiST-v02.patch

Patch didn't applied cleanly.
Patches were rebased onto current master.


*************** AUTHORS
*** 371,373 ****
--- 376,379 ----
  
      Teodor Sigaev <teodor@sigaev.ru>
      Oleg Bartunov <oleg@sai.msu.su>
+     Vlad Sterzhanov <gliderok@gmail.com>

I don't think it worth mentioning here GSoC student who made just dirty prototype and abandon this work just after final evaluation.
Student's mention was removed.

More generally, you switched SP-GiST from scan stack to pairing heap.  We should check if it introduces overhead to non-KNN scans. 
I decided to bring back scan stack for unordered scans.

Also some benchmarks comparing KNN SP-GIST vs KNN GiST would be now.

* 0005-Add-ordering-operators-to-SP-GiST-kd_point_ops-and-quad_point_ops-v02.patch

I faced following warning.

spgproc.c:32:10: warning: implicit declaration of function 'get_float8_nan' is invalid in C99 [-Wimplicit-function-declaration]
                return get_float8_nan();
                       ^
1 warning generated.
Fixed.

* 0006-Add-spg_compress-method-to-SP-GiST-v02.patch
* 0007-Add-SP-GiST-opclasses-poly_ops-and-circle_ops-v02.patch

I think this is out of scope for KNN SP-GiST.  This is very valuable, but completely separate feature.  And it should be posted and discussed separately. 
Compress method for SP-GiST with poly_ops already have been committed, so I left only ordering operators for polygons in the last 6th patch.

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: RFC: Add 'taint' field to pg_control.
Next
From: Tom Lane
Date:
Subject: Re: Two small patches for the isolationtester lexer