From 95b051aac56de68412a7b0635484e46eb4e24ad0 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 28 Jan 2015 09:36:11 +0900 Subject: [PATCH] Remove dead NULL-pointer checks in GiST code The key value passed to the same method is generated by either the compress, union or picksplit methods, but it happens that none of them actually pass a NULL pointer. Some compress methods do not check for a NULL pointer, what should have resulted in a crash. Note that gist_poly_compress() and gist_circle_compress() do check for a NULL, but they only return a NULL key if the input key is NULL, which cannot happen because the input comes from a table. This commit also removes a documentation note added in commit a0a3883 that has been introduced based on the fact that some routines of the same method were doing NULL-pointer checks, and adds an assertion in gistKeyIsEQ to ensure that no NULL keys are passed when calling the same method. --- contrib/btree_gist/btree_utils_num.c | 8 ++---- contrib/btree_gist/btree_utils_var.c | 10 ++----- doc/src/sgml/gist.sgml | 5 ---- src/backend/access/gist/gistproc.c | 56 ++++++++++++------------------------ src/backend/access/gist/gistutil.c | 3 ++ 5 files changed, 25 insertions(+), 57 deletions(-) diff --git a/contrib/btree_gist/btree_utils_num.c b/contrib/btree_gist/btree_utils_num.c index 505633c..7c5b911 100644 --- a/contrib/btree_gist/btree_utils_num.c +++ b/contrib/btree_gist/btree_utils_num.c @@ -147,13 +147,9 @@ gbt_num_same(const GBT_NUMKEY *a, const GBT_NUMKEY *b, const gbtree_ninfo *tinfo b2.lower = &(((GBT_NUMKEY *) b)[0]); b2.upper = &(((GBT_NUMKEY *) b)[tinfo->size]); - if ( - (*tinfo->f_eq) (b1.lower, b2.lower) && - (*tinfo->f_eq) (b1.upper, b2.upper) - ) - return TRUE; - return FALSE; + return ((*tinfo->f_eq) (b1.lower, b2.lower) && + (*tinfo->f_eq) (b1.upper, b2.upper)); } diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c index b7dd060..6ad3347 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -337,7 +337,6 @@ bool gbt_var_same(Datum d1, Datum d2, Oid collation, const gbtree_vinfo *tinfo) { - bool result; GBT_VARKEY *t1 = (GBT_VARKEY *) DatumGetPointer(d1); GBT_VARKEY *t2 = (GBT_VARKEY *) DatumGetPointer(d2); GBT_VARKEY_R r1, @@ -346,13 +345,8 @@ gbt_var_same(Datum d1, Datum d2, Oid collation, r1 = gbt_var_key_readable(t1); r2 = gbt_var_key_readable(t2); - if (t1 && t2) - result = ((*tinfo->f_cmp) (r1.lower, r2.lower, collation) == 0 && - (*tinfo->f_cmp) (r1.upper, r2.upper, collation) == 0); - else - result = (t1 == NULL && t2 == NULL); - - return result; + return ((*tinfo->f_cmp) (r1.lower, r2.lower, collation) == 0 && + (*tinfo->f_cmp) (r1.upper, r2.upper, collation) == 0); } diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml index 5de282b..f9644b0 100644 --- a/doc/src/sgml/gist.sgml +++ b/doc/src/sgml/gist.sgml @@ -498,11 +498,6 @@ my_compress(PG_FUNCTION_ARGS) course. - - Depending on your needs, you could also need to care about - compressing NULL values in there, storing for example - (Datum) 0 like gist_circle_compress does. - diff --git a/src/backend/access/gist/gistproc.c b/src/backend/access/gist/gistproc.c index 4decaa6..1df6357 100644 --- a/src/backend/access/gist/gistproc.c +++ b/src/backend/access/gist/gistproc.c @@ -1039,25 +1039,15 @@ gist_poly_compress(PG_FUNCTION_ARGS) if (entry->leafkey) { - retval = palloc(sizeof(GISTENTRY)); - if (DatumGetPointer(entry->key) != NULL) - { - POLYGON *in = DatumGetPolygonP(entry->key); - BOX *r; + POLYGON *in = DatumGetPolygonP(entry->key); + BOX *r; - r = (BOX *) palloc(sizeof(BOX)); - memcpy((void *) r, (void *) &(in->boundbox), sizeof(BOX)); - gistentryinit(*retval, PointerGetDatum(r), - entry->rel, entry->page, - entry->offset, FALSE); - - } - else - { - gistentryinit(*retval, (Datum) 0, - entry->rel, entry->page, - entry->offset, FALSE); - } + retval = palloc(sizeof(GISTENTRY)); + r = (BOX *) palloc(sizeof(BOX)); + memcpy((void *) r, (void *) &(in->boundbox), sizeof(BOX)); + gistentryinit(*retval, PointerGetDatum(r), + entry->rel, entry->page, + entry->offset, FALSE); } else retval = entry; @@ -1113,28 +1103,18 @@ gist_circle_compress(PG_FUNCTION_ARGS) if (entry->leafkey) { + CIRCLE *in = DatumGetCircleP(entry->key); + BOX *r; retval = palloc(sizeof(GISTENTRY)); - if (DatumGetCircleP(entry->key) != NULL) - { - CIRCLE *in = DatumGetCircleP(entry->key); - BOX *r; - - r = (BOX *) palloc(sizeof(BOX)); - r->high.x = in->center.x + in->radius; - r->low.x = in->center.x - in->radius; - r->high.y = in->center.y + in->radius; - r->low.y = in->center.y - in->radius; - gistentryinit(*retval, PointerGetDatum(r), - entry->rel, entry->page, - entry->offset, FALSE); - } - else - { - gistentryinit(*retval, (Datum) 0, - entry->rel, entry->page, - entry->offset, FALSE); - } + r = (BOX *) palloc(sizeof(BOX)); + r->high.x = in->center.x + in->radius; + r->low.x = in->center.x - in->radius; + r->high.y = in->center.y + in->radius; + r->low.y = in->center.y - in->radius; + gistentryinit(*retval, PointerGetDatum(r), + entry->rel, entry->page, + entry->offset, FALSE); } else retval = entry; diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index 38af0e0..a44cce8 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -276,6 +276,9 @@ gistKeyIsEQ(GISTSTATE *giststate, int attno, Datum a, Datum b) { bool result; + /* both keys cannot be NULL */ + Assert(DatumGetPointer(a) != NULL && DatumGetPointer(b) != NULL); + FunctionCall3Coll(&giststate->equalFn[attno], giststate->supportCollation[attno], a, b, -- 2.2.2