> 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.

Hmm, I was confused by phrase “create a uniform _internal_ representation” and 
actually internally cube stored “as is”. But probably i just misinterpret that.
So here is the updated version with old documentation restored.

Attachment: cube_distances.patch
Description: Binary data



> On 16 Dec 2015, at 16:46, Tomas Vondra <tomas.von...@2ndquadrant.com> 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:
>>> 
>>>   <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