Bug #14320 MBROverlaps performs faulty test
Submitted: 26 Oct 2005 9:07 Modified: 26 Jan 2006 2:42
Reporter: Tom Gidden Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:5.0.13, 4.1 OS:Any (any)
Assigned to: Alexey Botchkov

[26 Oct 2005 9:07] Tom Gidden
Description:
We are having a few problems with the MBROverlaps function in the spatial extensions.  On reading the source code to try to understand the function we found the following code:

  bool inner_point(double x, double y) const
  {
    /* The following should be safe, even if we compare doubles */
    return (xmin<x) && (xmax>x) && (ymin<y) && (ymax>x);
  }

(sql/spatial.h, lines 142-147)

The final comparison, (ymax>x), appears to be faulty.  The test should be (ymax>y).  This may account for some of the difficulties we are having with the function. 

How to repeat:
At the moment we do not have a test case for this.  The lack of documentation of MBROverlaps means that we are not sure of the expected behaviour (leading us to examine the source).

In particular, clarification of the difference between MBRIntersects and MBROverlaps would be useful.

Suggested fix:
*** spatial.h.orig   Wed Oct 26 09:42:50 2005
--- spatial.h  Wed Oct 26 10:05:23 2005
***************
*** 142,148 ****
    bool inner_point(double x, double y) const
    {
      /* The following should be safe, even if we compare doubles */
!     return (xmin<x) && (xmax>x) && (ymin<y) && (ymax>x);
    }
  
    int overlaps(const MBR *mbr)
--- 142,148 ----
    bool inner_point(double x, double y) const
    {
      /* The following should be safe, even if we compare doubles */
!     return (xmin<x) && (xmax>x) && (ymin<y) && (ymax>y);
    }
  
    int overlaps(const MBR *mbr)
[26 Oct 2005 10:01] Hartmut Holzgraefe
same problem in the 4.1 code base
[10 Dec 2005 13:49] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/59
[12 Jan 2006 12:25] Alexander Barkov
The fix seems correct. Ok to push.
Consider adding a test case.
[26 Jan 2006 2:42] Mike Hillyer
Documented in 4.1.18 changelog:

 <listitem>
        <para>
          The <literal>MBROverlaps</literal> GIS function returned
          incorrect results. (Bug #14320)
        </para>
      </listitem>