> On 16 Dec 2015, at 16:46, Tomas Vondra wrote: > > Hi, > > On 12/16/2015 01:26 PM, Stas Kelvich wrote: >> Hi, thanks for the review. >> >>> 1) (nitpicking) There seem to be some minor whitespace issues, i.e. >>> trailing spaces, empty lines being added/removed, etc. >> >> >> Fixed, I think >> >>> 2) one of the regression tests started to fail >>> >>> SELECT '-1e-700'::cube AS cube; >>> >>> This used to return (0) but now I get (-0). >> >> Actually that problem emerged because of the first problem. I had > extra whitespace in sql file and removed that whitespace from one of the > answers file (cube_1.sql), so diff with both cube.sql and cube_1.sql was > one line length and you saw diff with cube.sql. >> In all systems that available to me (osx/linux/freebsd) I saw that > right answers file is cube_1.sql. But in other OS’es you can get +/- 0 > or e27/e027. I edited that answers files manually, so there probably can > be some other typos. > > Ah! So that's why I couldn't quickly find the issue in the C code ... > >> >>> 3) I wonder why the docs were changed like this: >>> >>> >>> - 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. >>> + When corners coincide cube stores only one corner along with a >>> special flag in order to reduce size wasted. >>> >>> >>> Was the old behavior removed? I don't think so - it seems to behave >>> as before, so why to remove this information? Maybe it's not useful? >>> But then why add the bit about optimizing storage of points? >> >> I’ve edited it because the statement was mislead (or at least ambiguous) — cube_in function doesn’t swap coordinates. >> Simple way to see it: >>> select '(1,3),(3,1)'::cube; >> cube >> --------------- >> (1, 3),(3, 1) >> >> But LowerLeft-UpperRight representation should be (1,1),(3,3) > > I don't think that's what the comment says, actually. It rather refers to code like this: > > result = Min(LL_COORD(c, n - 1), UR_COORD(c, n - 1)); > > i.e. if you specifically ask for a particular corner (ll, in this case), you'll get the proper value. > > regards > > -- > Tomas Vondra http://www.2ndQuadrant.com > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services