Re: Incorrect behaviour when using a GiST index on points

I need someone to review this patch for 9.3.  We have already missed
fixing this for 9.2.

---------------------------------------------------------------------------

On Thu, Jun 21, 2012 at 10:53:43PM +0400, Alexander Korotkov wrote:
> On Wed, Feb 22, 2012 at 5:56 PM, Alexander Korotkov <aekorotkov@gmail.com>
> wrote:
> 
>     Attached patch fixes GiST behaviour without altering operators behaviour. 
> 
> 
> I think we definitely should apply this patch before 9.2 release, because it is
> a bug fix. Otherwise people will continue produce incorrect GiST indexes with
> in-core geometrical opclasses until 9.3. Patch is very simple and only changes
> few lines of code.
> 
> Any thoughts?
> 
> ------
> With best regards,
> Alexander Korotkov.

> *** a/src/backend/access/gist/gistproc.c
> --- b/src/backend/access/gist/gistproc.c
> ***************
> *** 836,842 **** gist_box_picksplit(PG_FUNCTION_ARGS)
>   }
>   
>   /*
> !  * Equality method
>    *
>    * This is used for both boxes and points.
>    */
> --- 836,843 ----
>   }
>   
>   /*
> !  * Equality method. Returns true only when boxes are exact same. We can't
> !  * ignore small extents because of index consistency.
>    *
>    * This is used for both boxes and points.
>    */
> ***************
> *** 848,856 **** gist_box_same(PG_FUNCTION_ARGS)
>       bool       *result = (bool *) PG_GETARG_POINTER(2);
>   
>       if (b1 && b2)
> !         *result = DatumGetBool(DirectFunctionCall2(box_same,
> !                                                    PointerGetDatum(b1),
> !                                                    PointerGetDatum(b2)));
>       else
>           *result = (b1 == NULL && b2 == NULL) ? TRUE : FALSE;
>       PG_RETURN_POINTER(result);
> --- 849,857 ----
>       bool       *result = (bool *) PG_GETARG_POINTER(2);
>   
>       if (b1 && b2)
> !         *result = (b1->low.x == b2->low.x && b1->low.y == b2->low.y && 
> !                    b1->high.x == b2->high.x && b1->high.y == b2->high.y)
> !                   ? TRUE : FALSE;
>       else
>           *result = (b1 == NULL && b2 == NULL) ? TRUE : FALSE;
>       PG_RETURN_POINTER(result);
> ***************
> *** 1326,1331 **** gist_point_consistent(PG_FUNCTION_ARGS)
> --- 1327,1333 ----
>       bool       *recheck = (bool *) PG_GETARG_POINTER(4);
>       bool        result;
>       StrategyNumber strategyGroup = strategy / GeoStrategyNumberOffset;
> +     BOX           *query, *key;
>   
>       switch (strategyGroup)
>       {
> ***************
> *** 1337,1348 **** gist_point_consistent(PG_FUNCTION_ARGS)
>               *recheck = false;
>               break;
>           case BoxStrategyNumberGroup:
> !             result = DatumGetBool(DirectFunctionCall5(
> !                                                       gist_box_consistent,
> !                                                       PointerGetDatum(entry),
> !                                                       PG_GETARG_DATUM(1),
> !                                       Int16GetDatum(RTOverlapStrategyNumber),
> !                                                0, PointerGetDatum(recheck)));
>               break;
>           case PolygonStrategyNumberGroup:
>               {
> --- 1339,1356 ----
>               *recheck = false;
>               break;
>           case BoxStrategyNumberGroup:
> !             /* 
> !              * This code repeats logic of on_ob which uses simple comparison
> !              * rather than FP* functions.
> !              */
> !             query = PG_GETARG_BOX_P(1);
> !             key = DatumGetBoxP(entry->key);
> !             
> !             *recheck = false;
> !             result = key->high.x >= query->low.x && 
> !                      key->low.x <= query->high.x &&
> !                      key->high.y >= query->low.y && 
> !                      key->low.y <= query->high.y;
>               break;
>           case PolygonStrategyNumberGroup:
>               {

> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +