Re: [HACKERS] [PATCH]: fix bug in SP-GiST box_ops - Mailing list pgsql-hackers

From Kyotaro HORIGUCHI
Subject Re: [HACKERS] [PATCH]: fix bug in SP-GiST box_ops
Date
Msg-id 20170130.180411.194328424.horiguchi.kyotaro@lab.ntt.co.jp
Whole thread Raw
In response to [HACKERS] [PATCH]: fix bug in SP-GiST box_ops  (Nikita Glukhov <n.gluhov@postgrespro.ru>)
Responses Re: [HACKERS] [PATCH]: fix bug in SP-GiST box_ops  (Nikita Glukhov <n.gluhov@postgrespro.ru>)
List pgsql-hackers
Hello,

At Mon, 30 Jan 2017 07:12:03 +0300, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote in
<9ea5b157-478c-8874-bc9b-050076b7d042@postgrespro.ru>
> Working on the tests for new SP-GiST opclasses for polygons and
> circles, I've
> found a bug in the SP-GiST box_ops (added in 9.6): some operators
> (&<, &>, $<|, |&>) have wrong tests in
> spg_box_quad_inner_consistent().
> This obviously leads to incorrect results of a SP-GiST index scan (see
> tests
> in the attached patch, their results were taken from a sequential
> scan).

Your problem is not necessarily evident for others.  Perhaps you
have to provide an explanation and/or a test case that describes
what is wrong for you so that others can get a clue for this
problem. Simpler test is better.


The test,

| +INSERT INTO quad_box_tbl
| +    SELECT i, box(point(x, y), point(x + w, y + h))
| +    FROM (SELECT i,
| +                random() * 1000 as x, random() * 1000 as y,
| +                random() * 20 as w, random() * 20 as h

is inserting quad_boxes generated using random numbers then,

| +SELECT count(*) FROM quad_box_tbl WHERE b <<  box '((100,200),(300,500))';
| + count 
| +-------
| +   891

counting them in this way is unstable. Even though this were
stable due to a fixed initial, this would be unacceptable, I
think. This kind of test should use nonrandom numbers.


Even though I don't understand this in depth, the following
change seems somewhat wrong in direction. Changing the second
argument type seems breaking the basis of the design.

| -lower2D(RangeBox *range_box, Range *query)
| +lower2D(kiRangeBox *range_box, double query)

reagrds,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center





pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: [HACKERS] Vacuum: allow usage of more than 1GB of work mem
Next
From: Amit Kapila
Date:
Subject: Re: [HACKERS] Parallel Index Scans