From: | Teodor Sigaev <teodor(at)sigaev(dot)ru> |
---|---|
To: | Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Sergey Konoplev <gray(dot)ru(at)gmail(dot)com>, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Cube extension kNN support |
Date: | 2015-12-01 14:52:39 |
Message-ID: | 565DB437.1070606@sigaev.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Patch looks good, but there ara some review notices:
1 gmake installcheck fails:
*** /.../pgsql/contrib/cube/expected/cube_1.out 2015-12-01 17:49:01.768764000
+0300
--- /.../pgsql/contrib/cube/results/cube.out 2015-12-01 17:49:12.190818000
+0300
***************
*** 1382,1388 ****
(1 row)
-- Test of distances
! --
SELECT cube_distance('(1,1)'::cube, '(4,5)'::cube);
cube_distance
---------------
--- 1382,1388 ----
(1 row)
-- Test of distances
! --
SELECT cube_distance('(1,1)'::cube, '(4,5)'::cube);
cube_distance
Seems, there a extra space at the end of string
2 Pls, don use in C-code magick constants like 'case 16:'. Use macros to define
some human-readable name (g_cube_distance())
3 Switch in g_cube_distance(): default switch path should generate a error. It
just simplifies a degbug process, may be in future.
4 Docs: pls, don't use a strings with unlimited length.
Stas Kelvich wrote:
> Hello.
>
> That is updated version of the patch with proper update scripts.
>
> Also i’ve noted that documentation states the wrong thing:
>
> “It does not matter which order the opposite corners of a cube are entered in. The cube functions automatically swap values if needed to create a uniform "lower left — upper right" internal representation."
>
> But in practice cubes stored "as is" and that leads to problems with getting cubes sorted along specific dimension directly from index.
> As a simplest workaround i’ve deleted that sentence from docs and implemented two coordinate getters -> and ~>. First one returns
> coordinate of cube as it stored, and second returns coordinate of cube normalised to (LL,UR)-form.
>
> Other way to fix thing is to force ’normalization’ while creating cube. But that can produce wrong sorts with already existing data.
>
>> On 09 Jul 2015, at 16:40, Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>>
>> Hi!
>>
>> On Sat, May 9, 2015 at 6:53 AM, Stas Kelvich <stas(dot)kelvich(at)gmail(dot)com> wrote:
>> Patch is pretty ready, last issue was about changed extension interface, so there should be migration script and version bump.
>> Attaching a version with all migration stuff.
>>
>> I can't see cube--1.0--1.1.sql in the patch. Did forget to include it?
>>
>> ------
>> Alexander Korotkov
>> Postgres Professional: http://www.postgrespro.com
>> The Russian Postgres Company
>
> Stas Kelvich
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>
>
>
--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2015-12-01 15:03:02 | Re: [PoC] Asynchronous execution again (which is not parallel) |
Previous Message | YUriy Zhuravlev | 2015-12-01 14:46:04 | Re: Some questions about the array. |