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

At Tue, 31 Jan 2017 14:38:39 +0300, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote in <1622dc9f-cecf-cee3-b71e-b2bf34649053(at)postgrespro(dot)ru>
> 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.

Thank you. I think this patch is already in a good shape. Let's
wait for a while for some commiter to pick up this. If you're
afraid that this might be forgotten, it might be better to
register this as an entry in the next commit fest.

> > 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.

GiST seems to split the space even for the first tuple. I saw
spg_box_quad_inner_consistent called several times on SELECT and
fortunately the scan for the tuple going into wrong node.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center