From ae20bf1740c53494e15fadfd8c2c6444032a3441 Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Fri, 22 Apr 2011 20:13:12 -0400 Subject: [PATCH] Make GIN and GIST pass the index collation to all their support functions. Experimentation with contrib/btree_gist shows that the majority of the GIST support functions potentially need collation information. Safest policy seems to be to pass it to all of them, instead of making assumptions about which ones could possibly need it. --- src/backend/access/gin/ginget.c | 6 ++-- src/backend/access/gin/ginscan.c | 17 ++++++------ src/backend/access/gin/ginutil.c | 21 +++++++------- src/backend/access/gist/gist.c | 17 ++++++++++++ src/backend/access/gist/gistsplit.c | 21 ++++++++------ src/backend/access/gist/gistutil.c | 43 +++++++++++++++++------------ src/include/access/gin_private.h | 4 +-- src/include/access/gist_private.h | 3 ++ 8 files changed, 82 insertions(+), 50 deletions(-) diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c index 227f84d9881..5b35b50034a 100644 --- a/src/backend/access/gin/ginget.c +++ b/src/backend/access/gin/ginget.c @@ -56,7 +56,7 @@ callConsistentFn(GinState *ginstate, GinScanKey key) key->recheckCurItem = true; return DatumGetBool(FunctionCall8Coll(&ginstate->consistentFn[key->attnum - 1], - ginstate->compareCollation[key->attnum - 1], + ginstate->supportCollation[key->attnum - 1], PointerGetDatum(key->entryRes), UInt16GetDatum(key->strategy), key->query, @@ -252,7 +252,7 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack, *---------- */ cmp = DatumGetInt32(FunctionCall4Coll(&btree->ginstate->comparePartialFn[attnum - 1], - btree->ginstate->compareCollation[attnum - 1], + btree->ginstate->supportCollation[attnum - 1], scanEntry->queryKey, idatum, UInt16GetDatum(scanEntry->strategy), @@ -1178,7 +1178,7 @@ matchPartialInPendingList(GinState *ginstate, Page page, *---------- */ cmp = DatumGetInt32(FunctionCall4Coll(&ginstate->comparePartialFn[entry->attnum - 1], - ginstate->compareCollation[entry->attnum - 1], + ginstate->supportCollation[entry->attnum - 1], entry->queryKey, datum[off - 1], UInt16GetDatum(entry->strategy), diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c index 37b08c0df62..d9f5b8c012e 100644 --- a/src/backend/access/gin/ginscan.c +++ b/src/backend/access/gin/ginscan.c @@ -305,14 +305,15 @@ ginNewScanKey(IndexScanDesc scan) /* OK to call the extractQueryFn */ queryValues = (Datum *) - DatumGetPointer(FunctionCall7(&so->ginstate.extractQueryFn[skey->sk_attno - 1], - skey->sk_argument, - PointerGetDatum(&nQueryValues), - UInt16GetDatum(skey->sk_strategy), - PointerGetDatum(&partial_matches), - PointerGetDatum(&extra_data), - PointerGetDatum(&nullFlags), - PointerGetDatum(&searchMode))); + DatumGetPointer(FunctionCall7Coll(&so->ginstate.extractQueryFn[skey->sk_attno - 1], + so->ginstate.supportCollation[skey->sk_attno - 1], + skey->sk_argument, + PointerGetDatum(&nQueryValues), + UInt16GetDatum(skey->sk_strategy), + PointerGetDatum(&partial_matches), + PointerGetDatum(&extra_data), + PointerGetDatum(&nullFlags), + PointerGetDatum(&searchMode))); /* * If bogus searchMode is returned, treat as GIN_SEARCH_MODE_ALL; note diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index a712331cf47..1ae51b10602 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -93,17 +93,17 @@ initGinState(GinState *state, Relation index) * while doing comparisons. However, we may have a collatable storage * type for a noncollatable indexed data type (for instance, hstore * uses text index entries). If there's no index collation then - * specify default collation in case the comparison function needs - * collation. This is harmless if the comparison function doesn't + * specify default collation in case the support functions need + * collation. This is harmless if the support functions don't * care about collation, so we just do it unconditionally. (We could * alternatively call get_typcollation, but that seems like expensive * overkill --- there aren't going to be any cases where a GIN storage * type has a nondefault collation.) */ if (OidIsValid(index->rd_indcollation[i])) - state->compareCollation[i] = index->rd_indcollation[i]; + state->supportCollation[i] = index->rd_indcollation[i]; else - state->compareCollation[i] = DEFAULT_COLLATION_OID; + state->supportCollation[i] = DEFAULT_COLLATION_OID; } } @@ -293,7 +293,7 @@ ginCompareEntries(GinState *ginstate, OffsetNumber attnum, /* both not null, so safe to call the compareFn */ return DatumGetInt32(FunctionCall2Coll(&ginstate->compareFn[attnum - 1], - ginstate->compareCollation[attnum - 1], + ginstate->supportCollation[attnum - 1], a, b)); } @@ -399,10 +399,11 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum, /* OK, call the opclass's extractValueFn */ nullFlags = NULL; /* in case extractValue doesn't set it */ entries = (Datum *) - DatumGetPointer(FunctionCall3(&ginstate->extractValueFn[attnum - 1], - value, - PointerGetDatum(nentries), - PointerGetDatum(&nullFlags))); + DatumGetPointer(FunctionCall3Coll(&ginstate->extractValueFn[attnum - 1], + ginstate->supportCollation[attnum - 1], + value, + PointerGetDatum(nentries), + PointerGetDatum(&nullFlags))); /* * Generate a placeholder if the item contained no keys. @@ -453,7 +454,7 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum, } arg.cmpDatumFunc = &ginstate->compareFn[attnum - 1]; - arg.collation = ginstate->compareCollation[attnum - 1]; + arg.collation = ginstate->supportCollation[attnum - 1]; arg.haveDups = false; qsort_arg(keydata, *nentries, sizeof(keyEntryData), cmpEntries, (void *) &arg); diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index fae3464600a..4881a7dd48b 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -17,6 +17,7 @@ #include "access/genam.h" #include "access/gist_private.h" #include "catalog/index.h" +#include "catalog/pg_collation.h" #include "miscadmin.h" #include "storage/bufmgr.h" #include "storage/indexfsm.h" @@ -1394,6 +1395,22 @@ initGISTstate(GISTSTATE *giststate, Relation index) CurrentMemoryContext); else giststate->distanceFn[i].fn_oid = InvalidOid; + + /* + * If the index column has a specified collation, we should honor that + * while doing comparisons. However, we may have a collatable storage + * type for a noncollatable indexed data type. If there's no index + * collation then specify default collation in case the support + * functions need collation. This is harmless if the support + * functions don't care about collation, so we just do it + * unconditionally. (We could alternatively call get_typcollation, + * but that seems like expensive overkill --- there aren't going to be + * any cases where a GIST storage type has a nondefault collation.) + */ + if (OidIsValid(index->rd_indcollation[i])) + giststate->supportCollation[i] = index->rd_indcollation[i]; + else + giststate->supportCollation[i] = DEFAULT_COLLATION_OID; } } diff --git a/src/backend/access/gist/gistsplit.c b/src/backend/access/gist/gistsplit.c index f65c493e980..bd846cecca6 100644 --- a/src/backend/access/gist/gistsplit.c +++ b/src/backend/access/gist/gistsplit.c @@ -325,16 +325,18 @@ genericPickSplit(GISTSTATE *giststate, GistEntryVector *entryvec, GIST_SPLITVEC evec->n = v->spl_nleft; memcpy(evec->vector, entryvec->vector + FirstOffsetNumber, sizeof(GISTENTRY) * evec->n); - v->spl_ldatum = FunctionCall2(&giststate->unionFn[attno], - PointerGetDatum(evec), - PointerGetDatum(&nbytes)); + v->spl_ldatum = FunctionCall2Coll(&giststate->unionFn[attno], + giststate->supportCollation[attno], + PointerGetDatum(evec), + PointerGetDatum(&nbytes)); evec->n = v->spl_nright; memcpy(evec->vector, entryvec->vector + FirstOffsetNumber + v->spl_nleft, sizeof(GISTENTRY) * evec->n); - v->spl_rdatum = FunctionCall2(&giststate->unionFn[attno], - PointerGetDatum(evec), - PointerGetDatum(&nbytes)); + v->spl_rdatum = FunctionCall2Coll(&giststate->unionFn[attno], + giststate->supportCollation[attno], + PointerGetDatum(evec), + PointerGetDatum(&nbytes)); } /* @@ -361,9 +363,10 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, int attno, GistSplitVec sv->spl_ldatum = v->spl_lattr[attno]; sv->spl_rdatum = v->spl_rattr[attno]; - FunctionCall2(&giststate->picksplitFn[attno], - PointerGetDatum(entryvec), - PointerGetDatum(sv)); + FunctionCall2Coll(&giststate->picksplitFn[attno], + giststate->supportCollation[attno], + PointerGetDatum(entryvec), + PointerGetDatum(sv)); if (sv->spl_nleft == 0 || sv->spl_nright == 0) { diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index e8bbd564c71..e61b676628b 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -207,9 +207,10 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke } /* Make union and store in attr array */ - attr[i] = FunctionCall2(&giststate->unionFn[i], - PointerGetDatum(evec), - PointerGetDatum(&attrsize)); + attr[i] = FunctionCall2Coll(&giststate->unionFn[i], + giststate->supportCollation[i], + PointerGetDatum(evec), + PointerGetDatum(&attrsize)); isnull[i] = FALSE; } @@ -271,9 +272,10 @@ gistMakeUnionKey(GISTSTATE *giststate, int attno, } *dstisnull = FALSE; - *dst = FunctionCall2(&giststate->unionFn[attno], - PointerGetDatum(evec), - PointerGetDatum(&dstsize)); + *dst = FunctionCall2Coll(&giststate->unionFn[attno], + giststate->supportCollation[attno], + PointerGetDatum(evec), + PointerGetDatum(&dstsize)); } } @@ -282,9 +284,10 @@ gistKeyIsEQ(GISTSTATE *giststate, int attno, Datum a, Datum b) { bool result; - FunctionCall3(&giststate->equalFn[attno], - a, b, - PointerGetDatum(&result)); + FunctionCall3Coll(&giststate->equalFn[attno], + giststate->supportCollation[attno], + a, b, + PointerGetDatum(&result)); return result; } @@ -442,8 +445,9 @@ gistdentryinit(GISTSTATE *giststate, int nkey, GISTENTRY *e, gistentryinit(*e, k, r, pg, o, l); dep = (GISTENTRY *) - DatumGetPointer(FunctionCall1(&giststate->decompressFn[nkey], - PointerGetDatum(e))); + DatumGetPointer(FunctionCall1Coll(&giststate->decompressFn[nkey], + giststate->supportCollation[nkey], + PointerGetDatum(e))); /* decompressFn may just return the given pointer */ if (dep != e) gistentryinit(*e, dep->key, dep->rel, dep->page, dep->offset, @@ -468,8 +472,9 @@ gistcentryinit(GISTSTATE *giststate, int nkey, gistentryinit(*e, k, r, pg, o, l); cep = (GISTENTRY *) - DatumGetPointer(FunctionCall1(&giststate->compressFn[nkey], - PointerGetDatum(e))); + DatumGetPointer(FunctionCall1Coll(&giststate->compressFn[nkey], + giststate->supportCollation[nkey], + PointerGetDatum(e))); /* compressFn may just return the given pointer */ if (cep != e) gistentryinit(*e, cep->key, cep->rel, cep->page, cep->offset, @@ -519,11 +524,13 @@ gistpenalty(GISTSTATE *giststate, int attno, { float penalty = 0.0; - if (giststate->penaltyFn[attno].fn_strict == FALSE || (isNullOrig == FALSE && isNullAdd == FALSE)) - FunctionCall3(&giststate->penaltyFn[attno], - PointerGetDatum(orig), - PointerGetDatum(add), - PointerGetDatum(&penalty)); + if (giststate->penaltyFn[attno].fn_strict == FALSE || + (isNullOrig == FALSE && isNullAdd == FALSE)) + FunctionCall3Coll(&giststate->penaltyFn[attno], + giststate->supportCollation[attno], + PointerGetDatum(orig), + PointerGetDatum(add), + PointerGetDatum(&penalty)); else if (isNullOrig && isNullAdd) penalty = 0.0; else diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index 06c6fa2f9c6..a79c003a9f1 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -303,8 +303,8 @@ typedef struct GinState FmgrInfo comparePartialFn[INDEX_MAX_KEYS]; /* optional method */ /* canPartialMatch[i] is true if comparePartialFn[i] is valid */ bool canPartialMatch[INDEX_MAX_KEYS]; - /* Collations to supply to the compareFns and comparePartialFns */ - Oid compareCollation[INDEX_MAX_KEYS]; + /* Collations to pass to the support functions */ + Oid supportCollation[INDEX_MAX_KEYS]; } GinState; /* XLog stuff */ diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index ecc188f7df7..77e3cb5aee8 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -41,6 +41,9 @@ typedef struct GISTSTATE FmgrInfo equalFn[INDEX_MAX_KEYS]; FmgrInfo distanceFn[INDEX_MAX_KEYS]; + /* Collations to pass to the support functions */ + Oid supportCollation[INDEX_MAX_KEYS]; + TupleDesc tupdesc; } GISTSTATE; -- GitLab