Re: [PATCH]: fix bug in SP-GiST box_ops

On 31.01.2017 13:04, Kyotaro HORIGUCHI wrote:
> The following comment,
>
>> /* Can any range from range_box to be overlower than this argument? */
>
> This might be better to be using the same wording to its users,
> for example the comment for overLeft4D is the following.
>
> | /* Can any rectangle from rect_box does not extend the right of this argument? */
>
> And the documentation uses the following sentence,
>
> https://www.postgresql.org/docs/current/static/functions-geometry.html
> | Does not extend to the right of?
>
> So I think "Can any range from range_box does not extend the
> upper of this argument?" is better than the overlower. What do
> you think?

I think "does not extend the upper" is better than unclear "overlower" too.
So I've corrected this comment in the attached v03 patch.

> I confirmed other functions in geo_spgist but found no bugs like this.

I found no bugs in other functions too.

> The minimal bad example for the '&<' case is the following.
>
> =# create table t (b box);
> =# create index on t using spgist(b);
> =# insert into t values ('((215, 305),(210,300))'::box);
> =# select * from t where b &< '((100,200),(300,400))'::box;
> b
> ---------------------
> (215,305),(210,300)
> (1 row)
>
> =# set enable_seqscan to false;
> =# select * from t where b &< '((100,200),(300,400))'::box;
> b
> ---
> (0 rows)

I don't know how you were able to reproduce this bug in spg_box_quad_inner_consistent()
using only one box because index tree must have at least one inner node (or 157 boxes)
in order to spg_box_quad_inner_consistent() began to be called. That's why existing
tests for box_ops didn't detect this bug: box_temp table has only 56 rows.

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company