Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-12-07 Thread Alexander Korotkov
On Wed, Nov 29, 2017 at 3:10 PM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> I'll try to provide meaningful review next week.
>>
>
> Cool, thanks.
>

Andrey Borodin complained through Telegram that he can't apply my patches
using "git apply".  After investigation of this problem, it appears that
the reason is that I still use git context diff setup according to our wiki
instruction (https://wiki.postgresql.org/wiki/Working_with_Git).  I've look
around some threads and realized that I'm probably the only guy who still
sends context diff patches.

Attached patchset in universal diff format.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


0001-cube-knn-fix-3.patch
Description: Binary data


0002-cube-knn-negative-coordinate-3.patch
Description: Binary data


Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-29 Thread Michael Paquier
On Wed, Nov 29, 2017 at 7:59 PM, Alexander Korotkov
 wrote:
> Sure, patch got some review.  I've no objection against moving this to the
> next commitfest though.

Please note that as this is qualified as a bug fix, I was not going to
mark it as returned with feedback or such. We want to keep track of
fixes that are bugs and potentially need back-patching.

> Since, these patches include bug fix, it's possible that someone will commit
> it before next commitfest.

Sure. Committers are free to commit patches they see suited for
integration even out of the CF time.
-- 
Michael



Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-29 Thread Alexander Korotkov
Hi!

On Wed, Nov 29, 2017 at 2:32 PM, Andrey Borodin 
wrote:

> 29 нояб. 2017 г., в 15:59, Alexander Korotkov 
> написал(а):
>
>
> Sure, patch got some review.  I've no objection against moving this to the
> next commitfest though.
> Since, these patches include bug fix, it's possible that someone will
> commit it before next commitfest.
>
> I've took a glance at the patch, here's what catches my eye in
> comments: corrdinate, dimenstions, descinding, stoty.
>

Thank you for catching these typos.  Rebased patchset with fixes typos is
attached.

I'll try to provide meaningful review next week.
>

Cool, thanks.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


0001-cube-knn-fix-2.patch
Description: Binary data


0002-cube-knn-negative-coordinate-2.patch
Description: Binary data


Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-29 Thread Andrey Borodin

> 29 нояб. 2017 г., в 15:59, Alexander Korotkov  
> написал(а):
> 
> 
> Sure, patch got some review.  I've no objection against moving this to the 
> next commitfest though.
> Since, these patches include bug fix, it's possible that someone will commit 
> it before next commitfest.

Hi!

I've took a glance at the patch, here's what catches my eye in comments: 
corrdinate, dimenstions, descinding, stoty.

I'll try to provide meaningful review next week.

Best regards, Andrey Borodin.

Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-29 Thread Alexander Korotkov
On Wed, Nov 29, 2017 at 1:30 PM, Tomas Vondra 
wrote:

> On 11/29/2017 06:13 AM, Michael Paquier wrote:
> > On Tue, Nov 21, 2017 at 7:07 AM, Alexander Korotkov
> >  wrote:
> >> On Mon, Nov 20, 2017 at 1:59 PM, Alexander Korotkov
> >>  wrote:
> >>>
> >>> On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra
> >>>  wrote:
> 
>  Seems fine to me, although perhaps it should be split into two parts.
>  One with the cube_coord_llur fixes, and then g_cube_distance changes
>  adding support for negative coordinates.
> >>>
> >>>
> >>> Thank you for your feedback.  I'll split this patch into two.
> >>
> >>
> >> Please, find two patches attached.
> >
> > This got no reviews, so moved to next CF.
> >
>
> That is somewhat inaccurate, I believe. I won't claim it received enough
> reviews, but it certainly received some. And splitting it into two does
> not invalidate that, I guess.


Sure, patch got some review.  I've no objection against moving this to the
next commitfest though.
Since, these patches include bug fix, it's possible that someone will
commit it before next commitfest.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-29 Thread Tomas Vondra


On 11/29/2017 06:13 AM, Michael Paquier wrote:
> On Tue, Nov 21, 2017 at 7:07 AM, Alexander Korotkov
>  wrote:
>> On Mon, Nov 20, 2017 at 1:59 PM, Alexander Korotkov
>>  wrote:
>>>
>>> On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra
>>>  wrote:

 Seems fine to me, although perhaps it should be split into two parts.
 One with the cube_coord_llur fixes, and then g_cube_distance changes
 adding support for negative coordinates.
>>>
>>>
>>> Thank you for your feedback.  I'll split this patch into two.
>>
>>
>> Please, find two patches attached.
> 
> This got no reviews, so moved to next CF.
> 

That is somewhat inaccurate, I believe. I won't claim it received enough
reviews, but it certainly received some. And splitting it into two does
not invalidate that, I guess.

regards

-- 
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-28 Thread Michael Paquier
On Tue, Nov 21, 2017 at 7:07 AM, Alexander Korotkov
 wrote:
> On Mon, Nov 20, 2017 at 1:59 PM, Alexander Korotkov
>  wrote:
>>
>> On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra
>>  wrote:
>>>
>>> Seems fine to me, although perhaps it should be split into two parts.
>>> One with the cube_coord_llur fixes, and then g_cube_distance changes
>>> adding support for negative coordinates.
>>
>>
>> Thank you for your feedback.  I'll split this patch into two.
>
>
> Please, find two patches attached.

This got no reviews, so moved to next CF.
-- 
Michael



Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-20 Thread Alexander Korotkov
On Mon, Nov 20, 2017 at 1:59 PM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra <
> tomas.von...@2ndquadrant.com> wrote:
>
>> Seems fine to me, although perhaps it should be split into two parts.
>> One with the cube_coord_llur fixes, and then g_cube_distance changes
>> adding support for negative coordinates.
>
>
> Thank you for your feedback.  I'll split this patch into two.
>

Please, find two patches attached.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


0001-cube-knn-fix.patch
Description: Binary data


0002-cube-knn-negative-coordinate.patch
Description: Binary data


Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-20 Thread Alexander Korotkov
Hi, Tomas!

On Mon, Nov 20, 2017 at 4:09 AM, Tomas Vondra 
wrote:

> Seems fine to me, although perhaps it should be split into two parts.
> One with the cube_coord_llur fixes, and then g_cube_distance changes
> adding support for negative coordinates.


Thank you for your feedback.  I'll split this patch into two.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Re: [HACKERS] CUBE seems a bit confused about ORDER BY

2017-11-19 Thread Tomas Vondra


On 10/30/2017 03:04 PM, Alexander Korotkov wrote:
> On Sun, Oct 29, 2017 at 1:30 AM, Alexander Korotkov
> mailto:a.korot...@postgrespro.ru>> wrote:
...
> 
> Thus, I heard no objection to my approach.  Attached patch changes ~>
> operator in the proposed way and fixes KNN-GiST search for that.  I'm
> going to register this patch to the nearest commitfest.
> 

Seems fine to me, although perhaps it should be split into two parts.
One with the cube_coord_llur fixes, and then g_cube_distance changes
adding support for negative coordinates.

regards

-- 
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services