From e378f82e0029a7a6fda9c655bd79d4404e566c0b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 5 Oct 2006 17:57:40 +0000 Subject: [PATCH] Make use of qsort_arg in several places that were formerly using klugy static variables. This avoids any risk of potential non-reentrancy, and in particular offers a much cleaner workaround for the Intel compiler bug that was affecting ginutil.c. --- contrib/btree_gist/btree_utils_var.c | 20 ++++----- contrib/tsearch2/rank.c | 22 +++++----- contrib/tsearch2/tsvector.c | 11 +++-- src/backend/access/gin/ginutil.c | 62 +++++++++++----------------- src/backend/commands/analyze.c | 45 ++++++++++---------- src/include/access/gin.h | 5 ++- 6 files changed, 77 insertions(+), 88 deletions(-) diff --git a/contrib/btree_gist/btree_utils_var.c b/contrib/btree_gist/btree_utils_var.c index 1f5ea12692..ae44e75491 100644 --- a/contrib/btree_gist/btree_utils_var.c +++ b/contrib/btree_gist/btree_utils_var.c @@ -439,18 +439,14 @@ gbt_var_penalty(float *res, const GISTENTRY *o, const GISTENTRY *n, const gbtree } -/* - * Fortunately, this sort comparison routine needn't be reentrant... - */ -static const gbtree_vinfo *gbt_vsrt_cmp_tinfo; - static int -gbt_vsrt_cmp(const void *a, const void *b) +gbt_vsrt_cmp(const void *a, const void *b, void *arg) { GBT_VARKEY_R ar = gbt_var_key_readable(((const Vsrt *) a)->t); GBT_VARKEY_R br = gbt_var_key_readable(((const Vsrt *) b)->t); + const gbtree_vinfo *tinfo = (const gbtree_vinfo *) arg; - return (*gbt_vsrt_cmp_tinfo->f_cmp) (ar.lower, br.lower); + return (*tinfo->f_cmp) (ar.lower, br.lower); } GIST_SPLITVEC * @@ -496,11 +492,11 @@ gbt_var_picksplit(const GistEntryVector *entryvec, GIST_SPLITVEC *v, const gbtre } /* sort */ - gbt_vsrt_cmp_tinfo = tinfo; - qsort((void *) &arr[FirstOffsetNumber], - maxoff - FirstOffsetNumber + 1, - sizeof(Vsrt), - gbt_vsrt_cmp); + qsort_arg((void *) &arr[FirstOffsetNumber], + maxoff - FirstOffsetNumber + 1, + sizeof(Vsrt), + gbt_vsrt_cmp, + (void *) tinfo); /* We do simply create two parts */ diff --git a/contrib/tsearch2/rank.c b/contrib/tsearch2/rank.c index 41e633580d..f8a7d5f2f0 100644 --- a/contrib/tsearch2/rank.c +++ b/contrib/tsearch2/rank.c @@ -122,14 +122,14 @@ find_wordentry(tsvector * t, QUERYTYPE * q, ITEM * item) } -static char *SortAndUniqOperand = NULL; - static int -compareITEM(const void *a, const void *b) +compareITEM(const void *a, const void *b, void *arg) { + char *operand = (char *) arg; + if ((*(ITEM **) a)->length == (*(ITEM **) b)->length) - return strncmp(SortAndUniqOperand + (*(ITEM **) a)->distance, - SortAndUniqOperand + (*(ITEM **) b)->distance, + return strncmp(operand + (*(ITEM **) a)->distance, + operand + (*(ITEM **) b)->distance, (*(ITEM **) b)->length); return ((*(ITEM **) a)->length > (*(ITEM **) b)->length) ? 1 : -1; @@ -158,15 +158,14 @@ SortAndUniqItems(char *operand, ITEM * item, int *size) if (*size < 2) return res; - SortAndUniqOperand = operand; - qsort(res, *size, sizeof(ITEM **), compareITEM); + qsort_arg(res, *size, sizeof(ITEM **), compareITEM, (void *) operand); ptr = res + 1; prevptr = res; while (ptr - res < *size) { - if (compareITEM((void *) ptr, (void *) prevptr) != 0) + if (compareITEM((void *) ptr, (void *) prevptr, (void *) operand) != 0) { prevptr++; *prevptr = *ptr; @@ -551,10 +550,11 @@ get_docrep(tsvector * txt, QUERYTYPE * query, int *doclen) int len = query->size * 4, cur = 0; DocRepresentation *doc; + char *operand; *(uint16 *) POSNULL = lengthof(POSNULL) - 1; doc = (DocRepresentation *) palloc(sizeof(DocRepresentation) * len); - SortAndUniqOperand = GETOPERAND(query); + operand = GETOPERAND(query); reset_istrue_flag(query); for (i = 0; i < query->size; i++) @@ -598,7 +598,9 @@ get_docrep(tsvector * txt, QUERYTYPE * query, int *doclen) for (k = 0; k < query->size; k++) { kptr = item + k; - if (k == i || (item[k].type == VAL && compareITEM(&kptr, &iptr) == 0)) + if (k == i || + (item[k].type == VAL && + compareITEM(&kptr, &iptr, operand) == 0)) { doc[cur].item[doc[cur].nitem] = item + k; doc[cur].nitem++; diff --git a/contrib/tsearch2/tsvector.c b/contrib/tsearch2/tsvector.c index a4a9d57d64..1abf1914e3 100644 --- a/contrib/tsearch2/tsvector.c +++ b/contrib/tsearch2/tsvector.c @@ -85,14 +85,14 @@ uniquePos(WordEntryPos * a, int4 l) return res + 1 - a; } -static char *BufferStr; static int -compareentry(const void *a, const void *b) +compareentry(const void *a, const void *b, void *arg) { + char *BufferStr = (char *) arg; + if (((WordEntryIN *) a)->entry.len == ((WordEntryIN *) b)->entry.len) { - return strncmp( - &BufferStr[((WordEntryIN *) a)->entry.pos], + return strncmp(&BufferStr[((WordEntryIN *) a)->entry.pos], &BufferStr[((WordEntryIN *) b)->entry.pos], ((WordEntryIN *) a)->entry.len); } @@ -117,8 +117,7 @@ uniqueentry(WordEntryIN * a, int4 l, char *buf, int4 *outbuflen) } ptr = a + 1; - BufferStr = buf; - qsort((void *) a, l, sizeof(WordEntryIN), compareentry); + qsort_arg((void *) a, l, sizeof(WordEntryIN), compareentry, (void *) buf); while (ptr - a < l) { diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index e64137a106..7b3c75db0d 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -8,7 +8,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/gin/ginutil.c,v 1.7 2006/10/04 00:29:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/gin/ginutil.c,v 1.8 2006/10/05 17:57:40 tgl Exp $ *------------------------------------------------------------------------- */ @@ -129,62 +129,48 @@ compareEntries(GinState *ginstate, Datum a, Datum b) ); } -static FmgrInfo *cmpDatumPtr = NULL; - -#if defined(__INTEL_COMPILER) && (defined(__ia64__) || defined(__ia64)) -/* - * Intel Compiler on Intel Itanium with -O2 has a bug around - * change static variable by user function called from - * libc func: it doesn't change. So mark it as volatile. - * - * It's a pity, but it's impossible to define optimization - * level here. - */ -#define VOLATILE volatile -#else -#define VOLATILE -#endif - -static bool VOLATILE needUnique = FALSE; +typedef struct +{ + FmgrInfo *cmpDatumFunc; + bool *needUnique; +} cmpEntriesData; static int -cmpEntries(const void *a, const void *b) +cmpEntries(const Datum *a, const Datum *b, cmpEntriesData *arg) { - int res = DatumGetInt32( - FunctionCall2( - cmpDatumPtr, - *(Datum *) a, - *(Datum *) b - ) - ); + int res = DatumGetInt32(FunctionCall2(arg->cmpDatumFunc, + *a, *b)); if (res == 0) - needUnique = TRUE; + *(arg->needUnique) = TRUE; return res; } Datum * -extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries) +extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries, + bool *needUnique) { Datum *entries; - entries = (Datum *) DatumGetPointer( - FunctionCall2( + entries = (Datum *) DatumGetPointer(FunctionCall2( &ginstate->extractValueFn, value, PointerGetDatum(nentries) - ) - ); + )); if (entries == NULL) *nentries = 0; + *needUnique = FALSE; if (*nentries > 1) { - cmpDatumPtr = &ginstate->compareFn; - needUnique = FALSE; - qsort(entries, *nentries, sizeof(Datum), cmpEntries); + cmpEntriesData arg; + + arg.cmpDatumFunc = &ginstate->compareFn; + arg.needUnique = needUnique; + qsort_arg(entries, *nentries, sizeof(Datum), + (qsort_arg_comparator) cmpEntries, (void *) &arg); } return entries; @@ -194,9 +180,11 @@ extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries) Datum * extractEntriesSU(GinState *ginstate, Datum value, uint32 *nentries) { - Datum *entries = extractEntriesS(ginstate, value, nentries); + bool needUnique; + Datum *entries = extractEntriesS(ginstate, value, nentries, + &needUnique); - if (*nentries > 1 && needUnique) + if (needUnique) { Datum *ptr, *res; diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 1ce768f046..cb538bdd42 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.99 2006/10/04 00:29:50 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.100 2006/10/05 17:57:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1294,11 +1294,12 @@ typedef struct int first; /* values[] index of first occurrence */ } ScalarMCVItem; - -/* context information for compare_scalars() */ -static FmgrInfo *datumCmpFn; -static SortFunctionKind datumCmpFnKind; -static int *datumCmpTupnoLink; +typedef struct +{ + FmgrInfo *cmpFn; + SortFunctionKind cmpFnKind; + int *tupnoLink; +} CompareScalarsContext; static void compute_minimal_stats(VacAttrStatsP stats, @@ -1309,7 +1310,7 @@ static void compute_scalar_stats(VacAttrStatsP stats, AnalyzeAttrFetchFunc fetchfunc, int samplerows, double totalrows); -static int compare_scalars(const void *a, const void *b); +static int compare_scalars(const void *a, const void *b, void *arg); static int compare_mcvs(const void *a, const void *b); @@ -1828,13 +1829,14 @@ compute_scalar_stats(VacAttrStatsP stats, num_hist, dups_cnt; int slot_idx = 0; + CompareScalarsContext cxt; /* Sort the collected values */ - datumCmpFn = &f_cmpfn; - datumCmpFnKind = cmpFnKind; - datumCmpTupnoLink = tupnoLink; - qsort((void *) values, values_cnt, - sizeof(ScalarItem), compare_scalars); + cxt.cmpFn = &f_cmpfn; + cxt.cmpFnKind = cmpFnKind; + cxt.tupnoLink = tupnoLink; + qsort_arg((void *) values, values_cnt, sizeof(ScalarItem), + compare_scalars, (void *) &cxt); /* * Now scan the values in order, find the most common ones, and also @@ -2183,35 +2185,36 @@ compute_scalar_stats(VacAttrStatsP stats, } /* - * qsort comparator for sorting ScalarItems + * qsort_arg comparator for sorting ScalarItems * - * Aside from sorting the items, we update the datumCmpTupnoLink[] array + * Aside from sorting the items, we update the tupnoLink[] array * whenever two ScalarItems are found to contain equal datums. The array * is indexed by tupno; for each ScalarItem, it contains the highest * tupno that that item's datum has been found to be equal to. This allows * us to avoid additional comparisons in compute_scalar_stats(). */ static int -compare_scalars(const void *a, const void *b) +compare_scalars(const void *a, const void *b, void *arg) { Datum da = ((ScalarItem *) a)->value; int ta = ((ScalarItem *) a)->tupno; Datum db = ((ScalarItem *) b)->value; int tb = ((ScalarItem *) b)->tupno; + CompareScalarsContext *cxt = (CompareScalarsContext *) arg; int32 compare; - compare = ApplySortFunction(datumCmpFn, datumCmpFnKind, + compare = ApplySortFunction(cxt->cmpFn, cxt->cmpFnKind, da, false, db, false); if (compare != 0) return compare; /* - * The two datums are equal, so update datumCmpTupnoLink[]. + * The two datums are equal, so update cxt->tupnoLink[]. */ - if (datumCmpTupnoLink[ta] < tb) - datumCmpTupnoLink[ta] = tb; - if (datumCmpTupnoLink[tb] < ta) - datumCmpTupnoLink[tb] = ta; + if (cxt->tupnoLink[ta] < tb) + cxt->tupnoLink[ta] = tb; + if (cxt->tupnoLink[tb] < ta) + cxt->tupnoLink[tb] = ta; /* * For equal datums, sort by tupno diff --git a/src/include/access/gin.h b/src/include/access/gin.h index 7035635e68..c37b367278 100644 --- a/src/include/access/gin.h +++ b/src/include/access/gin.h @@ -3,7 +3,7 @@ * header file for postgres inverted index access method implementation. * * Copyright (c) 2006, PostgreSQL Global Development Group - * $PostgreSQL: pgsql/src/include/access/gin.h,v 1.8 2006/10/04 00:30:06 momjian Exp $ + * $PostgreSQL: pgsql/src/include/access/gin.h,v 1.9 2006/10/05 17:57:40 tgl Exp $ *-------------------------------------------------------------------------- */ @@ -232,7 +232,8 @@ extern Buffer GinNewBuffer(Relation index); extern void GinInitBuffer(Buffer b, uint32 f); extern void GinInitPage(Page page, uint32 f, Size pageSize); extern int compareEntries(GinState *ginstate, Datum a, Datum b); -extern Datum *extractEntriesS(GinState *ginstate, Datum value, uint32 *nentries); +extern Datum *extractEntriesS(GinState *ginstate, Datum value, + uint32 *nentries, bool *needUnique); extern Datum *extractEntriesSU(GinState *ginstate, Datum value, uint32 *nentries); extern Page GinPageGetCopyPage(Page page); -- 2.40.0