Re: [HACKERS] [PATCH] kNN for SP-GiST

From: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
To: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [PATCH] kNN for SP-GiST
Date: 2018-02-28 21:58:42
Message-ID: 43c859f9-cb0d-1bd2-ba9f-c5e5b61185cc@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: 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(at)sigaev(dot)ru <mailto:teodor(at)sigaev(dot)ru>>
>       Oleg Bartunov <oleg(at)sai(dot)msu(dot)su <mailto:oleg(at)sai(dot)msu(dot)su>>
> +     Vlad Sterzhanov <gliderok(at)gmail(dot)com <mailto:gliderok(at)gmail(dot)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 Content-Type Size
0001-Export-box_fill-v03.patch text/x-patch 1.6 KB
0002-Extract-index_store_float8_orderby_distances-v03.patch text/x-patch 5.6 KB
0003-Extract-get_index_column_opclass-and-get_opclass_opfamily_and_input_type-v03.patch text/x-patch 5.1 KB
0004-Add-kNN-support-to-SP-GiST-v03.patch text/x-patch 50.3 KB
0005-Add-ordering-operators-to-SP-GiST-kd_point_ops-and-quad_point_ops-v03.patch text/x-patch 28.2 KB
0006-Add-ordering-operators-to-SP-GiST-poly_ops-v03.patch text/x-patch 9.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-02-28 21:59:34 Re: Two small patches for the isolationtester lexer
Previous Message Andres Freund 2018-02-28 21:43:11 RFC: Add 'taint' field to pg_control.