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.
> 3) I wonder why the docs were changed like this:
>
> <para>
> - It does not matter which order the opposite corners of a cube are
> - entered in. The <type>cube</> functions
> - automatically swap values if needed to create a uniform
> - <quote>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.
> </para>
>
> 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)
Updated patch attached.
cube_distances.patch
Description: Binary data
> On 15 Dec 2015, at 21:46, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > Hi, > > On 12/07/2015 03:47 PM, Stas Kelvich wrote: >> Hello, fixed. > > I've looked at the patch today, seems mostly fine to me. > > Three comments though: > > 1) (nitpicking) There seem to be some minor whitespace issues, i.e. > trailing spaces, empty lines being added/removed, etc. > > 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). As this query existed in > 1.0, it's probably due to change in the C code. Now sure where. > > 3) I wonder why the docs were changed like this: > > <para> > - It does not matter which order the opposite corners of a cube are > - entered in. The <type>cube</> functions > - automatically swap values if needed to create a uniform > - <quote>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. > </para> > > 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? > > regards > > -- > Tomas Vondra http://www.2ndQuadrant.com > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
