> On Sep 18, 2018, at 3:58 PM, Alexander Korotkov <a.korot...@postgrespro.ru> 
> wrote:
> 
> On Mon, Sep 17, 2018 at 12:42 PM Andrey Borodin <x4...@yandex-team.ru> wrote:
>>> 17 сент. 2018 г., в 2:03, Alexander Korotkov <a.korot...@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:

diff --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

Reply via email to