--- Stas Kelvich Postgres Professional: http://www.postgrespro.com Russian Postgres Company > On 01 Dec 2015, at 17:52, Teodor Sigaev wrote: > > 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 wrote: >>> >>> Hi! >>> >>> On Sat, May 9, 2015 at 6:53 AM, Stas Kelvich 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@sigaev.ru > WWW: http://www.sigaev.ru/ > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers