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