From: | Stas Kelvich <stas(dot)kelvich(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Cube extension kNN support |
Date: | 2015-12-16 12:26:35 |
Message-ID: | 32A833C3-9690-45CA-92AB-A16858F8A98A@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 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.
Attachment | Content-Type | Size |
---|---|---|
cube_distances.patch | application/octet-stream | 66.4 KB |
unknown_filename | text/plain | 1.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2015-12-16 12:34:25 | Re: [PoC] Asynchronous execution again (which is not parallel) |
Previous Message | Tomas Vondra | 2015-12-16 12:17:52 | Re: Proposal: custom compression methods |