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:

   <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 &mdash; 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)

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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to