Re: Cube extension kNN support

Поиск
Список
Период
Сортировка
От Stas Kelvich
Тема Re: Cube extension kNN support
Дата
Msg-id 32A833C3-9690-45CA-92AB-A16858F8A98A@gmail.com
обсуждение исходный текст
Ответ на Re: Cube extension kNN support  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Ответы Re: Cube extension kNN support  (Tomas Vondra <tomas.vondra@2ndquadrant.com>)
Список pgsql-hackers
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
whitespacefrom one of the answers file (cube_1.sql), so diff with both cube.sql and cube_1.sql was one line length and
yousaw 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
youcan 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.





> On 15 Dec 2015, at 21:46, Tomas Vondra <tomas.vondra@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


Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tomas Vondra
Дата:
Сообщение: Re: Proposal: custom compression methods
Следующее
От: Robert Haas
Дата:
Сообщение: Re: [PoC] Asynchronous execution again (which is not parallel)