Hi! On Fri, Jun 29, 2018 at 5:37 PM Nikita Glukhov <n.glu...@postgrespro.ru> wrote: > On 06.03.2018 17:30, David Steele wrote: > > > I agree with Andres. Pushing this patch to the next CF. > > Attached 4th version of the patches rebased onto the current master. > Nothing interesting has changed from the previous version.
I took a look to this patchset. In general, it looks good for me, but I've following notes about it. * We're going to replace scan stack with pairing heap not only for KNN-search, but for regular search as well. Did you verify that it doesn't cause regression for regular search case, because insertion into pairing heap might be slower than insertion into stack? One may say that it didn't caused regression in GiST, and that's why it shouldn't cause regression in SP-GiST. However, SP-GiST trees might be much higher and these performance aspects might be different. * I think null handling requires better explanation. Nulls are specially handled in pairingheap_SpGistSearchItem_cmp(). In the same time you explicitly set infinity distances for leaf nulls. You probably have reasons to implement it this way, but I think this should be explained. Also isnull property of SpGistSearchItem doesn't have comment. * I think KNN support should be briefly described in src/backed/access/spgist/README. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
