Re: [PATCH 0/3] Introduce spgist quadtree @<(point,circle) operator

сб, 2 мар. 2019 г. в 12:14, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>:
>
> Hi!

Thanks for reply. Comments and questions are below.

>
> On Fri, Feb 1, 2019 at 7:08 PM Matwey V. Kornilov
> <matwey(dot)kornilov(at)gmail(dot)com> wrote:
> > This patch series is to add support for spgist quadtree @<(point,circle)
> > operator. The first two patches are to refactor existing code before
> > implemention the new feature. The third commit is the actual implementation
> > provided with a set of simple unit tests.
>
> Cool!
>
> > Matwey V. Kornilov (3):
> > Introduce helper variable in spgquadtreeproc.c
> > Introduce spg_quad_inner_consistent_box_helper() in spgquadtreeproc.c
> > Add initial support for spgist quadtree @<(point,circle) operator
>
> At first, I have to note that it's not necessary to post every patch
> in separate message. It would be both easier and comfortable for
> readers if you just put your patches as multiple attachments to the
> same email message.

Ok, sorry, git send-email does embedded patches by default.
Would it be even easier for you next time to receive github repo url
and branch to pull from?

>
> Regarding the patchset itself
> * spg_quad_inner_consistent_circle_helper() definitely needs comments.

Ok

> * In PostgreSQL we require that index scan produce exactly same
> results as sequence scan. Can we ensure this is so for
> @<(point,circle) operator even in corner cases of rounding error?

I am not sure that fully understand the issue here.
The operator is registered to have amopfamily =
'spgist/quad_point_ops', how could we use it without the index to do
sequence scan?

> * In our coding style we have function name is the separate line from
> its return type.

Ok

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

--
With best regards,
Matwey V. Kornilov