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(at)gmail(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +