pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Mark Dilger <hornschnorter@gmail.com>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Nikita Glukhov <n.gluhov@postgrespro.ru>
Cc: David Steele <david@pgmasters.net>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] [PATCH] kNN for SP-GiST
Date: Thu, 27 Sep 2018 10:28:16 -0700
Message-ID: <B2AEFCD0-836D-4654-9D59-3DF616E0A6F3@gmail.com> (raw)
In-Reply-To: <CAPpHfdsqYeG4COHZfe_pn1=x0nTC41g-z7qfnse3qt4Lme+XFA@mail.gmail.com>
References: <570825e8-47d0-4732-2bf6-88d67d2d51c8@postgrespro.ru>
	<b985b66a-240c-03f0-df66-faa8b045551b@postgrespro.ru>
	<CAPpHfdvLXtuPNNR2w0PXuTkuR=ssySSgEPrjuExjq05OM0=LBw@mail.gmail.com>
	<43c859f9-cb0d-1bd2-ba9f-c5e5b61185cc@postgrespro.ru>
	<20180302063548.7ojyn56xd4ptc36v@alap3.anarazel.de>
	<e8a7852a-e049-207f-d745-9a0b0e80d89c@pgmasters.net>
	<6a09b16a-0b93-6fb1-c14a-7797477c5f38@postgrespro.ru>
	<CAPpHfdu6Wm4DSAp8Pvwq0uo7fCSzsbrNy7x2v5EKK_g4Nkjx1Q@mail.gmail.com>
	<c6957e2c-3052-4fce-55ac-2cf7bcfd0dd9@postgrespro.ru>
	<01432E05-D1AD-4D80-A7B4-149A299E906B@yandex-team.ru>
	<13EDC8D7-A7DA-47ED-80B8-B48E01023633@yandex-team.ru>
	<8d429bed-1a7f-d592-7e13-03e99defde03@postgrespro.ru>
	<5A8DAE0D-E4E8-48A6-BC8C-048A240875A0@yandex-team.ru>
	<944a7113-1886-ee4e-b8e6-df3e4dbca02e@postgrespro.ru>
	<F6E32265-5E11-461A-83C1-1927B28E8600@yandex-team.ru>
	<CAPpHfdstcdyKr49qvLkVmcAd_GyKB-Umk+Qx6-g5Ni_fB8w1mA@mail.gmail.com>
	<CAPpHfdvGtEzwRHqOUM67ixvqLo+ZbGhV+KEk_YMLZZM_WTCy0g@mail.gmail.com>
	<CAPpHfduBO0VQxoL2+E7zqWitD7dLStpD-UJSLZPVRO0P3fDi9g@mail.gmail.com>
	<CAPpHfdsDCPJ0v+MEhSsZmtt38KW3eYqXG0oodRv0OmieXQiiRw@mail.gmail.com>
	<CAPpHfdscCV83E0Ks4Hk=jEYQLA4sL--W_5ty=TT_xyZ0DL88_A@mail.gmail.com>
	<CAPpHfdtLbjKiPLgPHM25HGMs096bvmJDmT5E_s38f_2EQTfjpA@mail.gmail.com>
	<C4994532-08F0-4993-8B95-30FDEE2B0975@yandex-team.ru>
	<CAPpHfdsqYeG4COHZfe_pn1=x0nTC41g-z7qfnse3qt4Lme+XFA@mail.gmail.com>



> On Sep 18, 2018, at 3:58 PM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> 
> On Mon, Sep 17, 2018 at 12:42 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
>>> 17 сент. 2018 г., в 2:03, Alexander Korotkov <a.korotkov@postgrespro.ru> написал(а):
>>> 
>>> Also, it appears to me that it's OK to be a single patch
>> 
>> +1, ISTM that these 6 patches represent atomic unit of work.
> 
> Thank you, pushed.

One note on this commit,

in my fork, I have converted BoxPGetDatum from a macro to a static inline function,
and it shows a thinko in your commit, namely that in->leafDatum is of type (BOX *),
but is actually (as the name implies) already a Datum:

spgquadtreeproc.c:469:27: error: incompatible integer to pointer conversion passing 'Datum' (aka 'unsigned long') to parameter of type 'BOX *' [-Werror,-Wint-conversion]
                                                                                                        BoxPGetDatum(in->leafDatum), true,

I'm not claiming this creates any active bug, just that it would make more sense to
handle the typing cleanly.  Perhaps the following:


inline patch 1: src/backend/access/spgist/spgquadtreeproc.c (+1/-2 lines) download inline-1.patchdiff --git a/src/backend/access/spgist/spgquadtreeproc.c b/src/backend/access/spgist/spgquadtreeproc.c
index dee438a307..90cc776899 100644
--- a/src/backend/access/spgist/spgquadtreeproc.c
+++ b/src/backend/access/spgist/spgquadtreeproc.c
@@ -465,8 +465,7 @@ spg_quad_leaf_consistent(PG_FUNCTION_ARGS)
 
        if (res && in->norderbys > 0)
                /* ok, it passes -> let's compute the distances */
-               out->distances = spg_key_orderbys_distances(
-                                                                                                       BoxPGetDatum(in->leafDatum), true,
+               out->distances = spg_key_orderbys_distances(in->leafDatum, true,
                                                                                                        in->orderbys, in->norderbys);


mark



view thread (27+ messages)  latest in thread

Message-ID: <B2AEFCD0-836D-4654-9D59-3DF616E0A6F3@gmail.com>
Permalink:  ../B2AEFCD0-836D-4654-9D59-3DF616E0A6F3@gmail.com/
Also on:    postgresql.org/message-id/B2AEFCD0-836D-4654-9D59-3DF616E0A6F3@gmail.com

 ·  · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: hornschnorter@gmail.com, a.korotkov@postgrespro.ru, x4mmm@yandex-team.ru, n.gluhov@postgrespro.ru, david@pgmasters.net, andres@anarazel.de
  Subject: Re: [HACKERS] [PATCH] kNN for SP-GiST
  In-Reply-To: <B2AEFCD0-836D-4654-9D59-3DF616E0A6F3@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox