From: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
---|---|
To: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] kNN for SP-GiST |
Date: | 2017-03-09 13:52:57 |
Message-ID: | CAPpHfdvLXtuPNNR2w0PXuTkuR=ssySSgEPrjuExjq05OM0=LBw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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().
* 0004-Add-kNN-support-to-SP-GiST-v02.patch
Patch didn't applied cleanly.
patching file doc/src/sgml/spgist.sgml
patching file src/backend/access/spgist/Makefile
patching file src/backend/access/spgist/README
patching file src/backend/access/spgist/spgscan.c
Hunk #5 succeeded at 242 with fuzz 2.
Hunk #11 FAILED at 861.
1 out of 11 hunks FAILED -- saving rejects to file
src/backend/access/spgist/spgscan.c.rej
patching file src/backend/access/spgist/spgtextproc.c
patching file src/backend/access/spgist/spgutils.c
Hunk #3 succeeded at 65 (offset 1 line).
Hunk #4 succeeded at 916 (offset 1 line).
patching file src/backend/access/spgist/spgvalidate.c
patching file src/include/access/spgist.h
patching file src/include/access/spgist_private.h
Hunk #4 FAILED at 191.
Hunk #5 succeeded at 441 (offset -230 lines).
1 out of 5 hunks FAILED -- saving rejects to file
src/include/access/spgist_private.h.rej
I had to apply failed chunks manually. While trying to compile that I
faced compile error.
spgtextproc.c:89:7: error: no member named 'suppLen' in 'struct
spgConfigOut'
cfg->suppLen = 0; /* we don't need any supplimentary data */
~~~ ^
I noticed that this line was removed in the next patch. After removing it,
I faced following error.
make[4]: *** No rule to make target `spgproc.o', needed by `objfiles.txt'.
Stop.
After removing spgproc.o from src/backend/access/spgist/Makefile, I finally
succeed to compile it.
*************** AUTHORS
> *** 371,373 ****
> --- 376,379 ----
>
> Teodor Sigaev <teodor(at)sigaev(dot)ru>
> Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>
> + Vlad Sterzhanov <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.
More generally, you switched SP-GiST from scan stack to pairing heap. We
should check if it introduces overhead to non-KNN 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.
* 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.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-03-09 13:54:48 | Re: Automatic cleanup of oldest WAL segments with pg_receivexlog |
Previous Message | Peter Eisentraut | 2017-03-09 13:52:27 | Re: background sessions |