From 9aab83fc5039d83e84144b7bed3fb1d62a74ae78 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 13 May 2017 15:14:39 -0400 Subject: [PATCH] Redesign get_attstatsslot()/free_attstatsslot() for more safety and speed. The mess cleaned up in commit da0759600 is clear evidence that it's a bug hazard to expect the caller of get_attstatsslot()/free_attstatsslot() to provide the correct type OID for the array elements in the slot. Moreover, we weren't even getting any performance benefit from that, since get_attstatsslot() was extracting the real type OID from the array anyway. So we ought to get rid of that requirement; indeed, it would make more sense for get_attstatsslot() to pass back the type OID it found, in case the caller isn't sure what to expect, which is likely in binary- compatible-operator cases. Another problem with the current implementation is that if the stats array element type is pass-by-reference, we incur a palloc/memcpy/pfree cycle for each element. That seemed acceptable when the code was written because we were targeting O(10) array sizes --- but these days, stats arrays are almost always bigger than that, sometimes much bigger. We can save a significant number of cycles by doing one palloc/memcpy/pfree of the whole array. Indeed, in the now-probably-common case where the array is toasted, that happens anyway so this method is basically free. (Note: although the catcache code will inline any out-of-line toasted values, it doesn't decompress them. At the other end of the size range, it doesn't expand short-header datums either. In either case, DatumGetArrayTypeP would have to make a copy. We do end up using an extra array copy step if the element type is pass-by-value and the array length is neither small enough for a short header nor large enough to have suffered compression. But that seems like a very acceptable price for winning in pass-by-ref cases.) Hence, redesign to take these insights into account. While at it, convert to an API in which we fill a struct rather than passing a bunch of pointers to individual output arguments. That will make it less painful if we ever want further expansion of what get_attstatsslot can pass back. It's certainly arguable that this is new development and not something to push post-feature-freeze. However, I view it as primarily bug-proofing and therefore something that's better to have sooner not later. Since we aren't quite at beta phase yet, let's put it in. Discussion: https://postgr.es/m/16364.1494520862@sss.pgh.pa.us --- contrib/intarray/_int_selfuncs.c | 31 +- src/backend/executor/nodeHash.c | 25 +- src/backend/tsearch/ts_selfuncs.c | 18 +- src/backend/utils/adt/array_selfuncs.c | 85 ++-- src/backend/utils/adt/network_selfuncs.c | 214 +++++----- src/backend/utils/adt/rangetypes_selfuncs.c | 58 ++- src/backend/utils/adt/selfuncs.c | 417 ++++++++------------ src/backend/utils/cache/lsyscache.c | 147 ++++--- src/include/utils/lsyscache.h | 34 +- src/include/utils/selfuncs.h | 4 +- 10 files changed, 434 insertions(+), 599 deletions(-) diff --git a/contrib/intarray/_int_selfuncs.c b/contrib/intarray/_int_selfuncs.c index 9b4a22fe34..3d92025ba5 100644 --- a/contrib/intarray/_int_selfuncs.c +++ b/contrib/intarray/_int_selfuncs.c @@ -136,11 +136,7 @@ _int_matchsel(PG_FUNCTION_ARGS) int nmcelems = 0; float4 minfreq = 0.0; float4 nullfrac = 0.0; - Form_pg_statistic stats; - Datum *values = NULL; - int nvalues = 0; - float4 *numbers = NULL; - int nnumbers = 0; + AttStatsSlot sslot; /* * If expression is not "variable @@ something" or "something @@ variable" @@ -193,6 +189,8 @@ _int_matchsel(PG_FUNCTION_ARGS) */ if (HeapTupleIsValid(vardata.statsTuple)) { + Form_pg_statistic stats; + stats = (Form_pg_statistic) GETSTRUCT(vardata.statsTuple); nullfrac = stats->stanullfrac; @@ -200,29 +198,30 @@ _int_matchsel(PG_FUNCTION_ARGS) * For an int4 array, the default array type analyze function will * collect a Most Common Elements list, which is an array of int4s. */ - if (get_attstatsslot(vardata.statsTuple, - INT4OID, -1, + if (get_attstatsslot(&sslot, vardata.statsTuple, STATISTIC_KIND_MCELEM, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { + Assert(sslot.valuetype == INT4OID); + /* * There should be three more Numbers than Values, because the * last three (for intarray) cells are taken for minimal, maximal * and nulls frequency. Punt if not. */ - if (nnumbers == nvalues + 3) + if (sslot.nnumbers == sslot.nvalues + 3) { /* Grab the lowest frequency. */ - minfreq = numbers[nnumbers - (nnumbers - nvalues)]; + minfreq = sslot.numbers[sslot.nnumbers - (sslot.nnumbers - sslot.nvalues)]; - mcelems = values; - mcefreqs = numbers; - nmcelems = nvalues; + mcelems = sslot.values; + mcefreqs = sslot.numbers; + nmcelems = sslot.nvalues; } } } + else + memset(&sslot, 0, sizeof(sslot)); /* Process the logical expression in the query, using the stats */ selec = int_query_opr_selec(GETQUERY(query) + query->size - 1, @@ -231,7 +230,7 @@ _int_matchsel(PG_FUNCTION_ARGS) /* MCE stats count only non-null rows, so adjust for null rows. */ selec *= (1.0 - nullfrac); - free_attstatsslot(INT4OID, values, nvalues, numbers, nnumbers); + free_attstatsslot(&sslot); ReleaseVariableStats(vardata); CLAMP_PROBABILITY(selec); diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c index cfc6b96093..d9789d0719 100644 --- a/src/backend/executor/nodeHash.c +++ b/src/backend/executor/nodeHash.c @@ -1283,10 +1283,7 @@ static void ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) { HeapTupleData *statsTuple; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; /* Do nothing if planner didn't identify the outer relation's join key */ if (!OidIsValid(node->skewTable)) @@ -1305,19 +1302,17 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) if (!HeapTupleIsValid(statsTuple)) return; - if (get_attstatsslot(statsTuple, node->skewColType, node->skewColTypmod, + if (get_attstatsslot(&sslot, statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { double frac; int nbuckets; FmgrInfo *hashfunctions; int i; - if (mcvsToUse > nvalues) - mcvsToUse = nvalues; + if (mcvsToUse > sslot.nvalues) + mcvsToUse = sslot.nvalues; /* * Calculate the expected fraction of outer relation that will @@ -1326,11 +1321,10 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) */ frac = 0; for (i = 0; i < mcvsToUse; i++) - frac += numbers[i]; + frac += sslot.numbers[i]; if (frac < SKEW_MIN_OUTER_FRACTION) { - free_attstatsslot(node->skewColType, - values, nvalues, numbers, nnumbers); + free_attstatsslot(&sslot); ReleaseSysCache(statsTuple); return; } @@ -1392,7 +1386,7 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) int bucket; hashvalue = DatumGetUInt32(FunctionCall1(&hashfunctions[0], - values[i])); + sslot.values[i])); /* * While we have not hit a hole in the hashtable and have not hit @@ -1426,8 +1420,7 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse) hashtable->spacePeak = hashtable->spaceUsed; } - free_attstatsslot(node->skewColType, - values, nvalues, numbers, nnumbers); + free_attstatsslot(&sslot); } ReleaseSysCache(statsTuple); diff --git a/src/backend/tsearch/ts_selfuncs.c b/src/backend/tsearch/ts_selfuncs.c index 904d8848c8..046f543c01 100644 --- a/src/backend/tsearch/ts_selfuncs.c +++ b/src/backend/tsearch/ts_selfuncs.c @@ -163,28 +163,22 @@ tsquerysel(VariableStatData *vardata, Datum constval) if (HeapTupleIsValid(vardata->statsTuple)) { Form_pg_statistic stats; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; stats = (Form_pg_statistic) GETSTRUCT(vardata->statsTuple); /* MCELEM will be an array of TEXT elements for a tsvector column */ - if (get_attstatsslot(vardata->statsTuple, - TEXTOID, -1, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCELEM, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { /* * There is a most-common-elements slot for the tsvector Var, so * use that. */ - selec = mcelem_tsquery_selec(query, values, nvalues, - numbers, nnumbers); - free_attstatsslot(TEXTOID, values, nvalues, numbers, nnumbers); + selec = mcelem_tsquery_selec(query, sslot.values, sslot.nvalues, + sslot.numbers, sslot.nnumbers); + free_attstatsslot(&sslot); } else { diff --git a/src/backend/utils/adt/array_selfuncs.c b/src/backend/utils/adt/array_selfuncs.c index cfaf87335a..3ae6018c67 100644 --- a/src/backend/utils/adt/array_selfuncs.c +++ b/src/backend/utils/adt/array_selfuncs.c @@ -137,35 +137,22 @@ scalararraysel_containment(PlannerInfo *root, statistic_proc_security_check(&vardata, cmpfunc->fn_oid)) { Form_pg_statistic stats; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; - float4 *hist; - int nhist; + AttStatsSlot sslot; + AttStatsSlot hslot; stats = (Form_pg_statistic) GETSTRUCT(vardata.statsTuple); /* MCELEM will be an array of same type as element */ - if (get_attstatsslot(vardata.statsTuple, - elemtype, vardata.atttypmod, + if (get_attstatsslot(&sslot, vardata.statsTuple, STATISTIC_KIND_MCELEM, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { /* For ALL case, also get histogram of distinct-element counts */ if (useOr || - !get_attstatsslot(vardata.statsTuple, - elemtype, vardata.atttypmod, + !get_attstatsslot(&hslot, vardata.statsTuple, STATISTIC_KIND_DECHIST, InvalidOid, - NULL, - NULL, NULL, - &hist, &nhist)) - { - hist = NULL; - nhist = 0; - } + ATTSTATSSLOT_NUMBERS)) + memset(&hslot, 0, sizeof(hslot)); /* * For = ANY, estimate as var @> ARRAY[const]. @@ -173,22 +160,26 @@ scalararraysel_containment(PlannerInfo *root, * For = ALL, estimate as var <@ ARRAY[const]. */ if (useOr) - selec = mcelem_array_contain_overlap_selec(values, nvalues, - numbers, nnumbers, + selec = mcelem_array_contain_overlap_selec(sslot.values, + sslot.nvalues, + sslot.numbers, + sslot.nnumbers, &constval, 1, OID_ARRAY_CONTAINS_OP, cmpfunc); else - selec = mcelem_array_contained_selec(values, nvalues, - numbers, nnumbers, + selec = mcelem_array_contained_selec(sslot.values, + sslot.nvalues, + sslot.numbers, + sslot.nnumbers, &constval, 1, - hist, nhist, + hslot.numbers, + hslot.nnumbers, OID_ARRAY_CONTAINED_OP, cmpfunc); - if (hist) - free_attstatsslot(elemtype, NULL, 0, hist, nhist); - free_attstatsslot(elemtype, values, nvalues, numbers, nnumbers); + free_attstatsslot(&hslot); + free_attstatsslot(&sslot); } else { @@ -369,49 +360,35 @@ calc_arraycontsel(VariableStatData *vardata, Datum constval, statistic_proc_security_check(vardata, cmpfunc->fn_oid)) { Form_pg_statistic stats; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; - float4 *hist; - int nhist; + AttStatsSlot sslot; + AttStatsSlot hslot; stats = (Form_pg_statistic) GETSTRUCT(vardata->statsTuple); /* MCELEM will be an array of same type as column */ - if (get_attstatsslot(vardata->statsTuple, - elemtype, vardata->atttypmod, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCELEM, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { /* * For "array <@ const" case we also need histogram of distinct * element counts. */ if (operator != OID_ARRAY_CONTAINED_OP || - !get_attstatsslot(vardata->statsTuple, - elemtype, vardata->atttypmod, + !get_attstatsslot(&hslot, vardata->statsTuple, STATISTIC_KIND_DECHIST, InvalidOid, - NULL, - NULL, NULL, - &hist, &nhist)) - { - hist = NULL; - nhist = 0; - } + ATTSTATSSLOT_NUMBERS)) + memset(&hslot, 0, sizeof(hslot)); /* Use the most-common-elements slot for the array Var. */ selec = mcelem_array_selec(array, typentry, - values, nvalues, - numbers, nnumbers, - hist, nhist, + sslot.values, sslot.nvalues, + sslot.numbers, sslot.nnumbers, + hslot.numbers, hslot.nnumbers, operator, cmpfunc); - if (hist) - free_attstatsslot(elemtype, NULL, 0, hist, nhist); - free_attstatsslot(elemtype, values, nvalues, numbers, nnumbers); + free_attstatsslot(&hslot); + free_attstatsslot(&sslot); } else { diff --git a/src/backend/utils/adt/network_selfuncs.c b/src/backend/utils/adt/network_selfuncs.c index bcdd902ee8..1d29ecd521 100644 --- a/src/backend/utils/adt/network_selfuncs.c +++ b/src/backend/utils/adt/network_selfuncs.c @@ -88,10 +88,9 @@ networksel(PG_FUNCTION_ARGS) Selectivity selec, mcv_selec, non_mcv_selec; - Datum constvalue, - *hist_values; - int hist_nvalues; + Datum constvalue; Form_pg_statistic stats; + AttStatsSlot hslot; double sumcommon, nullfrac; FmgrInfo proc; @@ -146,22 +145,19 @@ networksel(PG_FUNCTION_ARGS) * non-MCV population that satisfies the clause. If we don't, apply the * default selectivity to that population. */ - if (get_attstatsslot(vardata.statsTuple, - vardata.atttype, vardata.atttypmod, + if (get_attstatsslot(&hslot, vardata.statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &hist_values, &hist_nvalues, - NULL, NULL)) + ATTSTATSSLOT_VALUES)) { int opr_codenum = inet_opr_codenum(operator); /* Commute if needed, so we can consider histogram to be on the left */ if (!varonleft) opr_codenum = -opr_codenum; - non_mcv_selec = inet_hist_value_sel(hist_values, hist_nvalues, + non_mcv_selec = inet_hist_value_sel(hslot.values, hslot.nvalues, constvalue, opr_codenum); - free_attstatsslot(vardata.atttype, hist_values, hist_nvalues, NULL, 0); + free_attstatsslot(&hslot); } else non_mcv_selec = DEFAULT_SEL(operator); @@ -277,42 +273,33 @@ networkjoinsel_inner(Oid operator, hist1_exists = false, hist2_exists = false; int opr_codenum; - int mcv1_nvalues, - mcv2_nvalues, - mcv1_nnumbers, - mcv2_nnumbers, - hist1_nvalues, - hist2_nvalues, - mcv1_length = 0, + int mcv1_length = 0, mcv2_length = 0; - Datum *mcv1_values, - *mcv2_values, - *hist1_values, - *hist2_values; - float4 *mcv1_numbers, - *mcv2_numbers; + AttStatsSlot mcv1_slot; + AttStatsSlot mcv2_slot; + AttStatsSlot hist1_slot; + AttStatsSlot hist2_slot; if (HeapTupleIsValid(vardata1->statsTuple)) { stats = (Form_pg_statistic) GETSTRUCT(vardata1->statsTuple); nullfrac1 = stats->stanullfrac; - mcv1_exists = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, vardata1->atttypmod, + mcv1_exists = get_attstatsslot(&mcv1_slot, vardata1->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &mcv1_values, &mcv1_nvalues, - &mcv1_numbers, &mcv1_nnumbers); - hist1_exists = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, vardata1->atttypmod, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); + hist1_exists = get_attstatsslot(&hist1_slot, vardata1->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &hist1_values, &hist1_nvalues, - NULL, NULL); + ATTSTATSSLOT_VALUES); /* Arbitrarily limit number of MCVs considered */ - mcv1_length = Min(mcv1_nvalues, MAX_CONSIDERED_ELEMS); + mcv1_length = Min(mcv1_slot.nvalues, MAX_CONSIDERED_ELEMS); if (mcv1_exists) - sumcommon1 = mcv_population(mcv1_numbers, mcv1_length); + sumcommon1 = mcv_population(mcv1_slot.numbers, mcv1_length); + } + else + { + memset(&mcv1_slot, 0, sizeof(mcv1_slot)); + memset(&hist1_slot, 0, sizeof(hist1_slot)); } if (HeapTupleIsValid(vardata2->statsTuple)) @@ -320,22 +307,21 @@ networkjoinsel_inner(Oid operator, stats = (Form_pg_statistic) GETSTRUCT(vardata2->statsTuple); nullfrac2 = stats->stanullfrac; - mcv2_exists = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, vardata2->atttypmod, + mcv2_exists = get_attstatsslot(&mcv2_slot, vardata2->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &mcv2_values, &mcv2_nvalues, - &mcv2_numbers, &mcv2_nnumbers); - hist2_exists = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, vardata2->atttypmod, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); + hist2_exists = get_attstatsslot(&hist2_slot, vardata2->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &hist2_values, &hist2_nvalues, - NULL, NULL); + ATTSTATSSLOT_VALUES); /* Arbitrarily limit number of MCVs considered */ - mcv2_length = Min(mcv2_nvalues, MAX_CONSIDERED_ELEMS); + mcv2_length = Min(mcv2_slot.nvalues, MAX_CONSIDERED_ELEMS); if (mcv2_exists) - sumcommon2 = mcv_population(mcv2_numbers, mcv2_length); + sumcommon2 = mcv_population(mcv2_slot.numbers, mcv2_length); + } + else + { + memset(&mcv2_slot, 0, sizeof(mcv2_slot)); + memset(&hist2_slot, 0, sizeof(hist2_slot)); } opr_codenum = inet_opr_codenum(operator); @@ -344,8 +330,10 @@ networkjoinsel_inner(Oid operator, * Calculate selectivity for MCV vs MCV matches. */ if (mcv1_exists && mcv2_exists) - selec += inet_mcv_join_sel(mcv1_values, mcv1_numbers, mcv1_length, - mcv2_values, mcv2_numbers, mcv2_length, + selec += inet_mcv_join_sel(mcv1_slot.values, mcv1_slot.numbers, + mcv1_length, + mcv2_slot.values, mcv2_slot.numbers, + mcv2_length, operator); /* @@ -355,13 +343,13 @@ networkjoinsel_inner(Oid operator, */ if (mcv1_exists && hist2_exists) selec += (1.0 - nullfrac2 - sumcommon2) * - inet_mcv_hist_sel(mcv1_values, mcv1_numbers, mcv1_length, - hist2_values, hist2_nvalues, + inet_mcv_hist_sel(mcv1_slot.values, mcv1_slot.numbers, mcv1_length, + hist2_slot.values, hist2_slot.nvalues, opr_codenum); if (mcv2_exists && hist1_exists) selec += (1.0 - nullfrac1 - sumcommon1) * - inet_mcv_hist_sel(mcv2_values, mcv2_numbers, mcv2_length, - hist1_values, hist1_nvalues, + inet_mcv_hist_sel(mcv2_slot.values, mcv2_slot.numbers, mcv2_length, + hist1_slot.values, hist1_slot.nvalues, -opr_codenum); /* @@ -371,8 +359,8 @@ networkjoinsel_inner(Oid operator, if (hist1_exists && hist2_exists) selec += (1.0 - nullfrac1 - sumcommon1) * (1.0 - nullfrac2 - sumcommon2) * - inet_hist_inclusion_join_sel(hist1_values, hist1_nvalues, - hist2_values, hist2_nvalues, + inet_hist_inclusion_join_sel(hist1_slot.values, hist1_slot.nvalues, + hist2_slot.values, hist2_slot.nvalues, opr_codenum); /* @@ -383,18 +371,10 @@ networkjoinsel_inner(Oid operator, selec = (1.0 - nullfrac1) * (1.0 - nullfrac2) * DEFAULT_SEL(operator); /* Release stats. */ - if (mcv1_exists) - free_attstatsslot(vardata1->atttype, mcv1_values, mcv1_nvalues, - mcv1_numbers, mcv1_nnumbers); - if (mcv2_exists) - free_attstatsslot(vardata2->atttype, mcv2_values, mcv2_nvalues, - mcv2_numbers, mcv2_nnumbers); - if (hist1_exists) - free_attstatsslot(vardata1->atttype, hist1_values, hist1_nvalues, - NULL, 0); - if (hist2_exists) - free_attstatsslot(vardata2->atttype, hist2_values, hist2_nvalues, - NULL, 0); + free_attstatsslot(&mcv1_slot); + free_attstatsslot(&mcv2_slot); + free_attstatsslot(&hist1_slot); + free_attstatsslot(&hist2_slot); return selec; } @@ -423,42 +403,33 @@ networkjoinsel_semi(Oid operator, int opr_codenum; FmgrInfo proc; int i, - mcv1_nvalues, - mcv2_nvalues, - mcv1_nnumbers, - mcv2_nnumbers, - hist1_nvalues, - hist2_nvalues, mcv1_length = 0, mcv2_length = 0; - Datum *mcv1_values, - *mcv2_values, - *hist1_values, - *hist2_values; - float4 *mcv1_numbers, - *mcv2_numbers; + AttStatsSlot mcv1_slot; + AttStatsSlot mcv2_slot; + AttStatsSlot hist1_slot; + AttStatsSlot hist2_slot; if (HeapTupleIsValid(vardata1->statsTuple)) { stats = (Form_pg_statistic) GETSTRUCT(vardata1->statsTuple); nullfrac1 = stats->stanullfrac; - mcv1_exists = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, vardata1->atttypmod, + mcv1_exists = get_attstatsslot(&mcv1_slot, vardata1->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &mcv1_values, &mcv1_nvalues, - &mcv1_numbers, &mcv1_nnumbers); - hist1_exists = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, vardata1->atttypmod, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); + hist1_exists = get_attstatsslot(&hist1_slot, vardata1->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &hist1_values, &hist1_nvalues, - NULL, NULL); + ATTSTATSSLOT_VALUES); /* Arbitrarily limit number of MCVs considered */ - mcv1_length = Min(mcv1_nvalues, MAX_CONSIDERED_ELEMS); + mcv1_length = Min(mcv1_slot.nvalues, MAX_CONSIDERED_ELEMS); if (mcv1_exists) - sumcommon1 = mcv_population(mcv1_numbers, mcv1_length); + sumcommon1 = mcv_population(mcv1_slot.numbers, mcv1_length); + } + else + { + memset(&mcv1_slot, 0, sizeof(mcv1_slot)); + memset(&hist1_slot, 0, sizeof(hist1_slot)); } if (HeapTupleIsValid(vardata2->statsTuple)) @@ -466,22 +437,21 @@ networkjoinsel_semi(Oid operator, stats = (Form_pg_statistic) GETSTRUCT(vardata2->statsTuple); nullfrac2 = stats->stanullfrac; - mcv2_exists = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, vardata2->atttypmod, + mcv2_exists = get_attstatsslot(&mcv2_slot, vardata2->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &mcv2_values, &mcv2_nvalues, - &mcv2_numbers, &mcv2_nnumbers); - hist2_exists = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, vardata2->atttypmod, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); + hist2_exists = get_attstatsslot(&hist2_slot, vardata2->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &hist2_values, &hist2_nvalues, - NULL, NULL); + ATTSTATSSLOT_VALUES); /* Arbitrarily limit number of MCVs considered */ - mcv2_length = Min(mcv2_nvalues, MAX_CONSIDERED_ELEMS); + mcv2_length = Min(mcv2_slot.nvalues, MAX_CONSIDERED_ELEMS); if (mcv2_exists) - sumcommon2 = mcv_population(mcv2_numbers, mcv2_length); + sumcommon2 = mcv_population(mcv2_slot.numbers, mcv2_length); + } + else + { + memset(&mcv2_slot, 0, sizeof(mcv2_slot)); + memset(&hist2_slot, 0, sizeof(hist2_slot)); } opr_codenum = inet_opr_codenum(operator); @@ -499,10 +469,11 @@ networkjoinsel_semi(Oid operator, { for (i = 0; i < mcv1_length; i++) { - selec += mcv1_numbers[i] * - inet_semi_join_sel(mcv1_values[i], - mcv2_exists, mcv2_values, mcv2_length, - hist2_exists, hist2_values, hist2_nvalues, + selec += mcv1_slot.numbers[i] * + inet_semi_join_sel(mcv1_slot.values[i], + mcv2_exists, mcv2_slot.values, mcv2_length, + hist2_exists, + hist2_slot.values, hist2_slot.nvalues, hist2_weight, &proc, opr_codenum); } @@ -519,21 +490,22 @@ networkjoinsel_semi(Oid operator, * * If there are too many histogram elements, decimate to limit runtime. */ - if (hist1_exists && hist1_nvalues > 2 && (mcv2_exists || hist2_exists)) + if (hist1_exists && hist1_slot.nvalues > 2 && (mcv2_exists || hist2_exists)) { double hist_selec_sum = 0.0; int k, n; - k = (hist1_nvalues - 3) / MAX_CONSIDERED_ELEMS + 1; + k = (hist1_slot.nvalues - 3) / MAX_CONSIDERED_ELEMS + 1; n = 0; - for (i = 1; i < hist1_nvalues - 1; i += k) + for (i = 1; i < hist1_slot.nvalues - 1; i += k) { hist_selec_sum += - inet_semi_join_sel(hist1_values[i], - mcv2_exists, mcv2_values, mcv2_length, - hist2_exists, hist2_values, hist2_nvalues, + inet_semi_join_sel(hist1_slot.values[i], + mcv2_exists, mcv2_slot.values, mcv2_length, + hist2_exists, + hist2_slot.values, hist2_slot.nvalues, hist2_weight, &proc, opr_codenum); n++; @@ -550,18 +522,10 @@ networkjoinsel_semi(Oid operator, selec = (1.0 - nullfrac1) * (1.0 - nullfrac2) * DEFAULT_SEL(operator); /* Release stats. */ - if (mcv1_exists) - free_attstatsslot(vardata1->atttype, mcv1_values, mcv1_nvalues, - mcv1_numbers, mcv1_nnumbers); - if (mcv2_exists) - free_attstatsslot(vardata2->atttype, mcv2_values, mcv2_nvalues, - mcv2_numbers, mcv2_nnumbers); - if (hist1_exists) - free_attstatsslot(vardata1->atttype, hist1_values, hist1_nvalues, - NULL, 0); - if (hist2_exists) - free_attstatsslot(vardata2->atttype, hist2_values, hist2_nvalues, - NULL, 0); + free_attstatsslot(&mcv1_slot); + free_attstatsslot(&mcv2_slot); + free_attstatsslot(&hist1_slot); + free_attstatsslot(&hist2_slot); return selec; } diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/utils/adt/rangetypes_selfuncs.c index dbf1929a0d..c4c549658d 100644 --- a/src/backend/utils/adt/rangetypes_selfuncs.c +++ b/src/backend/utils/adt/rangetypes_selfuncs.c @@ -239,25 +239,21 @@ calc_rangesel(TypeCacheEntry *typcache, VariableStatData *vardata, if (HeapTupleIsValid(vardata->statsTuple)) { Form_pg_statistic stats; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; stats = (Form_pg_statistic) GETSTRUCT(vardata->statsTuple); null_frac = stats->stanullfrac; /* Try to get fraction of empty ranges */ - if (get_attstatsslot(vardata->statsTuple, - FLOAT8OID, -1, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM, InvalidOid, - NULL, - NULL, NULL, - &numbers, &nnumbers)) + ATTSTATSSLOT_NUMBERS)) { - if (nnumbers != 1) + if (sslot.nnumbers != 1) elog(ERROR, "invalid empty fraction statistic"); /* shouldn't happen */ - empty_frac = numbers[0]; - free_attstatsslot(FLOAT8OID, NULL, 0, numbers, nnumbers); + empty_frac = sslot.numbers[0]; + free_attstatsslot(&sslot); } else { @@ -374,10 +370,9 @@ static double calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, RangeType *constval, Oid operator) { - Datum *hist_values; + AttStatsSlot hslot; + AttStatsSlot lslot; int nhist; - Datum *length_hist_values = NULL; - int length_nhist = 0; RangeBound *hist_lower; RangeBound *hist_upper; int i; @@ -397,23 +392,21 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, /* Try to get histogram of ranges */ if (!(HeapTupleIsValid(vardata->statsTuple) && - get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + get_attstatsslot(&hslot, vardata->statsTuple, STATISTIC_KIND_BOUNDS_HISTOGRAM, InvalidOid, - NULL, - &hist_values, &nhist, - NULL, NULL))) + ATTSTATSSLOT_VALUES))) return -1.0; /* * Convert histogram of ranges into histograms of its lower and upper * bounds. */ + nhist = hslot.nvalues; hist_lower = (RangeBound *) palloc(sizeof(RangeBound) * nhist); hist_upper = (RangeBound *) palloc(sizeof(RangeBound) * nhist); for (i = 0; i < nhist; i++) { - range_deserialize(typcache, DatumGetRangeType(hist_values[i]), + range_deserialize(typcache, DatumGetRangeType(hslot.values[i]), &hist_lower[i], &hist_upper[i], &empty); /* The histogram should not contain any empty ranges */ if (empty) @@ -425,27 +418,25 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, operator == OID_RANGE_CONTAINED_OP) { if (!(HeapTupleIsValid(vardata->statsTuple) && - get_attstatsslot(vardata->statsTuple, - FLOAT8OID, -1, + get_attstatsslot(&lslot, vardata->statsTuple, STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM, InvalidOid, - NULL, - &length_hist_values, &length_nhist, - NULL, NULL))) + ATTSTATSSLOT_VALUES))) { - free_attstatsslot(vardata->atttype, hist_values, nhist, NULL, 0); + free_attstatsslot(&hslot); return -1.0; } /* check that it's a histogram, not just a dummy entry */ - if (length_nhist < 2) + if (lslot.nvalues < 2) { - free_attstatsslot(FLOAT8OID, - length_hist_values, length_nhist, NULL, 0); - free_attstatsslot(vardata->atttype, hist_values, nhist, NULL, 0); + free_attstatsslot(&lslot); + free_attstatsslot(&hslot); return -1.0; } } + else + memset(&lslot, 0, sizeof(lslot)); /* Extract the bounds of the constant value. */ range_deserialize(typcache, constval, &const_lower, &const_upper, &empty); @@ -545,7 +536,7 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, hist_selec = calc_hist_selectivity_contains(typcache, &const_lower, &const_upper, hist_lower, nhist, - length_hist_values, length_nhist); + lslot.values, lslot.nvalues); break; case OID_RANGE_CONTAINED_OP: @@ -570,7 +561,7 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, hist_selec = calc_hist_selectivity_contained(typcache, &const_lower, &const_upper, hist_lower, nhist, - length_hist_values, length_nhist); + lslot.values, lslot.nvalues); } break; @@ -580,9 +571,8 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata, break; } - free_attstatsslot(FLOAT8OID, - length_hist_values, length_nhist, NULL, 0); - free_attstatsslot(vardata->atttype, hist_values, nhist, NULL, 0); + free_attstatsslot(&lslot); + free_attstatsslot(&hslot); return hist_selec; } diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 9b157f4456..6c4cef9fb3 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -299,10 +299,7 @@ var_eq_const(VariableStatData *vardata, Oid operator, (opfuncoid = get_opcode(operator)))) { Form_pg_statistic stats; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; bool match = false; int i; @@ -315,30 +312,27 @@ var_eq_const(VariableStatData *vardata, Oid operator, * don't like this, maybe you shouldn't be using eqsel for your * operator...) */ - if (get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { FmgrInfo eqproc; fmgr_info(opfuncoid, &eqproc); - for (i = 0; i < nvalues; i++) + for (i = 0; i < sslot.nvalues; i++) { /* be careful to apply operator right way 'round */ if (varonleft) match = DatumGetBool(FunctionCall2Coll(&eqproc, DEFAULT_COLLATION_OID, - values[i], + sslot.values[i], constval)); else match = DatumGetBool(FunctionCall2Coll(&eqproc, DEFAULT_COLLATION_OID, constval, - values[i])); + sslot.values[i])); if (match) break; } @@ -346,9 +340,7 @@ var_eq_const(VariableStatData *vardata, Oid operator, else { /* no most-common-value info available */ - values = NULL; - numbers = NULL; - i = nvalues = nnumbers = 0; + i = 0; /* keep compiler quiet */ } if (match) @@ -357,7 +349,7 @@ var_eq_const(VariableStatData *vardata, Oid operator, * Constant is "=" to this common value. We know selectivity * exactly (or as exactly as ANALYZE could calculate it, anyway). */ - selec = numbers[i]; + selec = sslot.numbers[i]; } else { @@ -369,8 +361,8 @@ var_eq_const(VariableStatData *vardata, Oid operator, double sumcommon = 0.0; double otherdistinct; - for (i = 0; i < nnumbers; i++) - sumcommon += numbers[i]; + for (i = 0; i < sslot.nnumbers; i++) + sumcommon += sslot.numbers[i]; selec = 1.0 - sumcommon - stats->stanullfrac; CLAMP_PROBABILITY(selec); @@ -379,7 +371,8 @@ var_eq_const(VariableStatData *vardata, Oid operator, * all the not-common values share this remaining fraction * equally, so we divide by the number of other distinct values. */ - otherdistinct = get_variable_numdistinct(vardata, &isdefault) - nnumbers; + otherdistinct = get_variable_numdistinct(vardata, &isdefault) - + sslot.nnumbers; if (otherdistinct > 1) selec /= otherdistinct; @@ -387,12 +380,11 @@ var_eq_const(VariableStatData *vardata, Oid operator, * Another cross-check: selectivity shouldn't be estimated as more * than the least common "most common value". */ - if (nnumbers > 0 && selec > numbers[nnumbers - 1]) - selec = numbers[nnumbers - 1]; + if (sslot.nnumbers > 0 && selec > sslot.numbers[sslot.nnumbers - 1]) + selec = sslot.numbers[sslot.nnumbers - 1]; } - free_attstatsslot(vardata->atttype, values, nvalues, - numbers, nnumbers); + free_attstatsslot(&sslot); } else { @@ -435,8 +427,7 @@ var_eq_non_const(VariableStatData *vardata, Oid operator, { Form_pg_statistic stats; double ndistinct; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; stats = (Form_pg_statistic) GETSTRUCT(vardata->statsTuple); @@ -459,16 +450,13 @@ var_eq_non_const(VariableStatData *vardata, Oid operator, * Cross-check: selectivity should never be estimated as more than the * most common value's. */ - if (get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - NULL, NULL, - &numbers, &nnumbers)) + ATTSTATSSLOT_NUMBERS)) { - if (nnumbers > 0 && selec > numbers[0]) - selec = numbers[0]; - free_attstatsslot(vardata->atttype, NULL, 0, numbers, nnumbers); + if (sslot.nnumbers > 0 && selec > sslot.numbers[0]) + selec = sslot.numbers[0]; + free_attstatsslot(&sslot); } } else @@ -620,10 +608,7 @@ mcv_selectivity(VariableStatData *vardata, FmgrInfo *opproc, { double mcv_selec, sumcommon; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; int i; mcv_selec = 0.0; @@ -631,29 +616,25 @@ mcv_selectivity(VariableStatData *vardata, FmgrInfo *opproc, if (HeapTupleIsValid(vardata->statsTuple) && statistic_proc_security_check(vardata, opproc->fn_oid) && - get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers)) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS)) { - for (i = 0; i < nvalues; i++) + for (i = 0; i < sslot.nvalues; i++) { if (varonleft ? DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, - values[i], + sslot.values[i], constval)) : DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, constval, - values[i]))) - mcv_selec += numbers[i]; - sumcommon += numbers[i]; + sslot.values[i]))) + mcv_selec += sslot.numbers[i]; + sumcommon += sslot.numbers[i]; } - free_attstatsslot(vardata->atttype, values, nvalues, - numbers, nnumbers); + free_attstatsslot(&sslot); } *sumcommonp = sumcommon; @@ -699,8 +680,7 @@ histogram_selectivity(VariableStatData *vardata, FmgrInfo *opproc, int *hist_size) { double result; - Datum *values; - int nvalues; + AttStatsSlot sslot; /* check sanity of parameters */ Assert(n_skip >= 0); @@ -708,37 +688,34 @@ histogram_selectivity(VariableStatData *vardata, FmgrInfo *opproc, if (HeapTupleIsValid(vardata->statsTuple) && statistic_proc_security_check(vardata, opproc->fn_oid) && - get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &values, &nvalues, - NULL, NULL)) + ATTSTATSSLOT_VALUES)) { - *hist_size = nvalues; - if (nvalues >= min_hist_size) + *hist_size = sslot.nvalues; + if (sslot.nvalues >= min_hist_size) { int nmatch = 0; int i; - for (i = n_skip; i < nvalues - n_skip; i++) + for (i = n_skip; i < sslot.nvalues - n_skip; i++) { if (varonleft ? DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, - values[i], + sslot.values[i], constval)) : DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, constval, - values[i]))) + sslot.values[i]))) nmatch++; } - result = ((double) nmatch) / ((double) (nvalues - 2 * n_skip)); + result = ((double) nmatch) / ((double) (sslot.nvalues - 2 * n_skip)); } else result = -1; - free_attstatsslot(vardata->atttype, values, nvalues, NULL, 0); + free_attstatsslot(&sslot); } else { @@ -768,9 +745,7 @@ ineq_histogram_selectivity(PlannerInfo *root, Datum constval, Oid consttype) { double hist_selec; - Oid hist_op; - Datum *values; - int nvalues; + AttStatsSlot sslot; hist_selec = -1.0; @@ -786,14 +761,11 @@ ineq_histogram_selectivity(PlannerInfo *root, */ if (HeapTupleIsValid(vardata->statsTuple) && statistic_proc_security_check(vardata, opproc->fn_oid) && - get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - &hist_op, - &values, &nvalues, - NULL, NULL)) + ATTSTATSSLOT_VALUES)) { - if (nvalues > 1) + if (sslot.nvalues > 1) { /* * Use binary search to find proper location, ie, the first slot @@ -812,7 +784,7 @@ ineq_histogram_selectivity(PlannerInfo *root, */ double histfrac; int lobound = 0; /* first possible slot to search */ - int hibound = nvalues; /* last+1 slot to search */ + int hibound = sslot.nvalues; /* last+1 slot to search */ bool have_end = false; /* @@ -821,12 +793,12 @@ ineq_histogram_selectivity(PlannerInfo *root, * one of them to be updated, so we deal with that within the * loop.) */ - if (nvalues == 2) + if (sslot.nvalues == 2) have_end = get_actual_variable_range(root, vardata, - hist_op, - &values[0], - &values[1]); + sslot.staop, + &sslot.values[0], + &sslot.values[1]); while (lobound < hibound) { @@ -838,22 +810,22 @@ ineq_histogram_selectivity(PlannerInfo *root, * histogram entry, first try to replace it with the actual * current min or max (unless we already did so above). */ - if (probe == 0 && nvalues > 2) + if (probe == 0 && sslot.nvalues > 2) have_end = get_actual_variable_range(root, vardata, - hist_op, - &values[0], + sslot.staop, + &sslot.values[0], NULL); - else if (probe == nvalues - 1 && nvalues > 2) + else if (probe == sslot.nvalues - 1 && sslot.nvalues > 2) have_end = get_actual_variable_range(root, vardata, - hist_op, + sslot.staop, NULL, - &values[probe]); + &sslot.values[probe]); ltcmp = DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, - values[probe], + sslot.values[probe], constval)); if (isgt) ltcmp = !ltcmp; @@ -868,7 +840,7 @@ ineq_histogram_selectivity(PlannerInfo *root, /* Constant is below lower histogram boundary. */ histfrac = 0.0; } - else if (lobound >= nvalues) + else if (lobound >= sslot.nvalues) { /* Constant is above upper histogram boundary. */ histfrac = 1.0; @@ -889,7 +861,7 @@ ineq_histogram_selectivity(PlannerInfo *root, * interpolation within this bin. */ if (convert_to_scalar(constval, consttype, &val, - values[i - 1], values[i], + sslot.values[i - 1], sslot.values[i], vardata->vartype, &low, &high)) { @@ -936,7 +908,7 @@ ineq_histogram_selectivity(PlannerInfo *root, * binfrac partial bin below the constant. */ histfrac = (double) (i - 1) + binfrac; - histfrac /= (double) (nvalues - 1); + histfrac /= (double) (sslot.nvalues - 1); } /* @@ -964,7 +936,7 @@ ineq_histogram_selectivity(PlannerInfo *root, } } - free_attstatsslot(vardata->atttype, values, nvalues, NULL, 0); + free_attstatsslot(&sslot); } return hist_selec; @@ -1517,21 +1489,15 @@ booltestsel(PlannerInfo *root, BoolTestType booltesttype, Node *arg, { Form_pg_statistic stats; double freq_null; - Datum *values; - int nvalues; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; stats = (Form_pg_statistic) GETSTRUCT(vardata.statsTuple); freq_null = stats->stanullfrac; - if (get_attstatsslot(vardata.statsTuple, - vardata.atttype, vardata.atttypmod, + if (get_attstatsslot(&sslot, vardata.statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &values, &nvalues, - &numbers, &nnumbers) - && nnumbers > 0) + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS) + && sslot.nnumbers > 0) { double freq_true; double freq_false; @@ -1539,10 +1505,10 @@ booltestsel(PlannerInfo *root, BoolTestType booltesttype, Node *arg, /* * Get first MCV frequency and derive frequency for true. */ - if (DatumGetBool(values[0])) - freq_true = numbers[0]; + if (DatumGetBool(sslot.values[0])) + freq_true = sslot.numbers[0]; else - freq_true = 1.0 - numbers[0] - freq_null; + freq_true = 1.0 - sslot.numbers[0] - freq_null; /* * Next derive frequency for false. Then use these as appropriate @@ -1583,8 +1549,7 @@ booltestsel(PlannerInfo *root, BoolTestType booltesttype, Node *arg, break; } - free_attstatsslot(vardata.atttype, values, nvalues, - numbers, nnumbers); + free_attstatsslot(&sslot); } else { @@ -2274,34 +2239,26 @@ eqjoinsel_inner(Oid operator, Form_pg_statistic stats1 = NULL; Form_pg_statistic stats2 = NULL; bool have_mcvs1 = false; - Datum *values1 = NULL; - int nvalues1 = 0; - float4 *numbers1 = NULL; - int nnumbers1 = 0; bool have_mcvs2 = false; - Datum *values2 = NULL; - int nvalues2 = 0; - float4 *numbers2 = NULL; - int nnumbers2 = 0; + AttStatsSlot sslot1; + AttStatsSlot sslot2; nd1 = get_variable_numdistinct(vardata1, &isdefault1); nd2 = get_variable_numdistinct(vardata2, &isdefault2); opfuncoid = get_opcode(operator); + memset(&sslot1, 0, sizeof(sslot1)); + memset(&sslot2, 0, sizeof(sslot2)); + if (HeapTupleIsValid(vardata1->statsTuple)) { /* note we allow use of nullfrac regardless of security check */ stats1 = (Form_pg_statistic) GETSTRUCT(vardata1->statsTuple); if (statistic_proc_security_check(vardata1, opfuncoid)) - have_mcvs1 = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, - vardata1->atttypmod, - STATISTIC_KIND_MCV, - InvalidOid, - NULL, - &values1, &nvalues1, - &numbers1, &nnumbers1); + have_mcvs1 = get_attstatsslot(&sslot1, vardata1->statsTuple, + STATISTIC_KIND_MCV, InvalidOid, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); } if (HeapTupleIsValid(vardata2->statsTuple)) @@ -2309,14 +2266,9 @@ eqjoinsel_inner(Oid operator, /* note we allow use of nullfrac regardless of security check */ stats2 = (Form_pg_statistic) GETSTRUCT(vardata2->statsTuple); if (statistic_proc_security_check(vardata2, opfuncoid)) - have_mcvs2 = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, - vardata2->atttypmod, - STATISTIC_KIND_MCV, - InvalidOid, - NULL, - &values2, &nvalues2, - &numbers2, &nnumbers2); + have_mcvs2 = get_attstatsslot(&sslot2, vardata2->statsTuple, + STATISTIC_KIND_MCV, InvalidOid, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); } if (have_mcvs1 && have_mcvs2) @@ -2351,8 +2303,8 @@ eqjoinsel_inner(Oid operator, nmatches; fmgr_info(opfuncoid, &eqproc); - hasmatch1 = (bool *) palloc0(nvalues1 * sizeof(bool)); - hasmatch2 = (bool *) palloc0(nvalues2 * sizeof(bool)); + hasmatch1 = (bool *) palloc0(sslot1.nvalues * sizeof(bool)); + hasmatch2 = (bool *) palloc0(sslot2.nvalues * sizeof(bool)); /* * Note we assume that each MCV will match at most one member of the @@ -2362,21 +2314,21 @@ eqjoinsel_inner(Oid operator, */ matchprodfreq = 0.0; nmatches = 0; - for (i = 0; i < nvalues1; i++) + for (i = 0; i < sslot1.nvalues; i++) { int j; - for (j = 0; j < nvalues2; j++) + for (j = 0; j < sslot2.nvalues; j++) { if (hasmatch2[j]) continue; if (DatumGetBool(FunctionCall2Coll(&eqproc, DEFAULT_COLLATION_OID, - values1[i], - values2[j]))) + sslot1.values[i], + sslot2.values[j]))) { hasmatch1[i] = hasmatch2[j] = true; - matchprodfreq += numbers1[i] * numbers2[j]; + matchprodfreq += sslot1.numbers[i] * sslot2.numbers[j]; nmatches++; break; } @@ -2385,22 +2337,22 @@ eqjoinsel_inner(Oid operator, CLAMP_PROBABILITY(matchprodfreq); /* Sum up frequencies of matched and unmatched MCVs */ matchfreq1 = unmatchfreq1 = 0.0; - for (i = 0; i < nvalues1; i++) + for (i = 0; i < sslot1.nvalues; i++) { if (hasmatch1[i]) - matchfreq1 += numbers1[i]; + matchfreq1 += sslot1.numbers[i]; else - unmatchfreq1 += numbers1[i]; + unmatchfreq1 += sslot1.numbers[i]; } CLAMP_PROBABILITY(matchfreq1); CLAMP_PROBABILITY(unmatchfreq1); matchfreq2 = unmatchfreq2 = 0.0; - for (i = 0; i < nvalues2; i++) + for (i = 0; i < sslot2.nvalues; i++) { if (hasmatch2[i]) - matchfreq2 += numbers2[i]; + matchfreq2 += sslot2.numbers[i]; else - unmatchfreq2 += numbers2[i]; + unmatchfreq2 += sslot2.numbers[i]; } CLAMP_PROBABILITY(matchfreq2); CLAMP_PROBABILITY(unmatchfreq2); @@ -2425,15 +2377,15 @@ eqjoinsel_inner(Oid operator, * MCVs plus non-MCV values. */ totalsel1 = matchprodfreq; - if (nd2 > nvalues2) - totalsel1 += unmatchfreq1 * otherfreq2 / (nd2 - nvalues2); + if (nd2 > sslot2.nvalues) + totalsel1 += unmatchfreq1 * otherfreq2 / (nd2 - sslot2.nvalues); if (nd2 > nmatches) totalsel1 += otherfreq1 * (otherfreq2 + unmatchfreq2) / (nd2 - nmatches); /* Same estimate from the point of view of relation 2. */ totalsel2 = matchprodfreq; - if (nd1 > nvalues1) - totalsel2 += unmatchfreq2 * otherfreq1 / (nd1 - nvalues1); + if (nd1 > sslot1.nvalues) + totalsel2 += unmatchfreq2 * otherfreq1 / (nd1 - sslot1.nvalues); if (nd1 > nmatches) totalsel2 += otherfreq2 * (otherfreq1 + unmatchfreq1) / (nd1 - nmatches); @@ -2478,12 +2430,8 @@ eqjoinsel_inner(Oid operator, selec /= nd2; } - if (have_mcvs1) - free_attstatsslot(vardata1->atttype, values1, nvalues1, - numbers1, nnumbers1); - if (have_mcvs2) - free_attstatsslot(vardata2->atttype, values2, nvalues2, - numbers2, nnumbers2); + free_attstatsslot(&sslot1); + free_attstatsslot(&sslot2); return selec; } @@ -2508,21 +2456,18 @@ eqjoinsel_semi(Oid operator, Oid opfuncoid; Form_pg_statistic stats1 = NULL; bool have_mcvs1 = false; - Datum *values1 = NULL; - int nvalues1 = 0; - float4 *numbers1 = NULL; - int nnumbers1 = 0; bool have_mcvs2 = false; - Datum *values2 = NULL; - int nvalues2 = 0; - float4 *numbers2 = NULL; - int nnumbers2 = 0; + AttStatsSlot sslot1; + AttStatsSlot sslot2; nd1 = get_variable_numdistinct(vardata1, &isdefault1); nd2 = get_variable_numdistinct(vardata2, &isdefault2); opfuncoid = OidIsValid(operator) ? get_opcode(operator) : InvalidOid; + memset(&sslot1, 0, sizeof(sslot1)); + memset(&sslot2, 0, sizeof(sslot2)); + /* * We clamp nd2 to be not more than what we estimate the inner relation's * size to be. This is intuitively somewhat reasonable since obviously @@ -2561,27 +2506,18 @@ eqjoinsel_semi(Oid operator, /* note we allow use of nullfrac regardless of security check */ stats1 = (Form_pg_statistic) GETSTRUCT(vardata1->statsTuple); if (statistic_proc_security_check(vardata1, opfuncoid)) - have_mcvs1 = get_attstatsslot(vardata1->statsTuple, - vardata1->atttype, - vardata1->atttypmod, - STATISTIC_KIND_MCV, - InvalidOid, - NULL, - &values1, &nvalues1, - &numbers1, &nnumbers1); + have_mcvs1 = get_attstatsslot(&sslot1, vardata1->statsTuple, + STATISTIC_KIND_MCV, InvalidOid, + ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); } if (HeapTupleIsValid(vardata2->statsTuple) && statistic_proc_security_check(vardata2, opfuncoid)) { - have_mcvs2 = get_attstatsslot(vardata2->statsTuple, - vardata2->atttype, - vardata2->atttypmod, - STATISTIC_KIND_MCV, - InvalidOid, - NULL, - &values2, &nvalues2, - &numbers2, &nnumbers2); + have_mcvs2 = get_attstatsslot(&sslot2, vardata2->statsTuple, + STATISTIC_KIND_MCV, InvalidOid, + ATTSTATSSLOT_VALUES); + /* note: currently don't need stanumbers from RHS */ } if (have_mcvs1 && have_mcvs2 && OidIsValid(operator)) @@ -2607,15 +2543,15 @@ eqjoinsel_semi(Oid operator, /* * The clamping above could have resulted in nd2 being less than - * nvalues2; in which case, we assume that precisely the nd2 most - * common values in the relation will appear in the join input, and so - * compare to only the first nd2 members of the MCV list. Of course - * this is frequently wrong, but it's the best bet we can make. + * sslot2.nvalues; in which case, we assume that precisely the nd2 + * most common values in the relation will appear in the join input, + * and so compare to only the first nd2 members of the MCV list. Of + * course this is frequently wrong, but it's the best bet we can make. */ - clamped_nvalues2 = Min(nvalues2, nd2); + clamped_nvalues2 = Min(sslot2.nvalues, nd2); fmgr_info(opfuncoid, &eqproc); - hasmatch1 = (bool *) palloc0(nvalues1 * sizeof(bool)); + hasmatch1 = (bool *) palloc0(sslot1.nvalues * sizeof(bool)); hasmatch2 = (bool *) palloc0(clamped_nvalues2 * sizeof(bool)); /* @@ -2625,7 +2561,7 @@ eqjoinsel_semi(Oid operator, * and because the math wouldn't add up... */ nmatches = 0; - for (i = 0; i < nvalues1; i++) + for (i = 0; i < sslot1.nvalues; i++) { int j; @@ -2635,8 +2571,8 @@ eqjoinsel_semi(Oid operator, continue; if (DatumGetBool(FunctionCall2Coll(&eqproc, DEFAULT_COLLATION_OID, - values1[i], - values2[j]))) + sslot1.values[i], + sslot2.values[j]))) { hasmatch1[i] = hasmatch2[j] = true; nmatches++; @@ -2646,10 +2582,10 @@ eqjoinsel_semi(Oid operator, } /* Sum up frequencies of matched MCVs */ matchfreq1 = 0.0; - for (i = 0; i < nvalues1; i++) + for (i = 0; i < sslot1.nvalues; i++) { if (hasmatch1[i]) - matchfreq1 += numbers1[i]; + matchfreq1 += sslot1.numbers[i]; } CLAMP_PROBABILITY(matchfreq1); pfree(hasmatch1); @@ -2704,12 +2640,8 @@ eqjoinsel_semi(Oid operator, selec = 0.5 * (1.0 - nullfrac1); } - if (have_mcvs1) - free_attstatsslot(vardata1->atttype, values1, nvalues1, - numbers1, nnumbers1); - if (have_mcvs2) - free_attstatsslot(vardata2->atttype, values2, nvalues2, - numbers2, nnumbers2); + free_attstatsslot(&sslot1); + free_attstatsslot(&sslot2); return selec; } @@ -3629,8 +3561,7 @@ estimate_hash_bucketsize(PlannerInfo *root, Node *hashkey, double nbuckets) mcvfreq, avgfreq; bool isdefault; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; examine_variable(root, hashkey, 0, &vardata); @@ -3689,20 +3620,16 @@ estimate_hash_bucketsize(PlannerInfo *root, Node *hashkey, double nbuckets) if (HeapTupleIsValid(vardata.statsTuple)) { - if (get_attstatsslot(vardata.statsTuple, - vardata.atttype, vardata.atttypmod, + if (get_attstatsslot(&sslot, vardata.statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - NULL, NULL, - &numbers, &nnumbers)) + ATTSTATSSLOT_NUMBERS)) { /* * The first MCV stat is for the most common value. */ - if (nnumbers > 0) - mcvfreq = numbers[0]; - free_attstatsslot(vardata.atttype, NULL, 0, - numbers, nnumbers); + if (sslot.nnumbers > 0) + mcvfreq = sslot.numbers[0]; + free_attstatsslot(&sslot); } } @@ -4572,7 +4499,7 @@ get_join_variables(PlannerInfo *root, List *args, SpecialJoinInfo *sjinfo, * freefunc: pointer to a function to release statsTuple with. * vartype: exposed type of the expression; this should always match * the declared input type of the operator we are estimating for. - * atttype, atttypmod: type data to pass to get_attstatsslot(). This is + * atttype, atttypmod: actual type/typmod of the "var" expression. This is * commonly the same as the exposed type of the variable argument, * but can be different in binary-compatible-type cases. * isunique: TRUE if we were able to match the var to a unique index or a @@ -5125,8 +5052,7 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, int16 typLen; bool typByVal; Oid opfuncoid; - Datum *values; - int nvalues; + AttStatsSlot sslot; int i; /* @@ -5167,29 +5093,23 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, * the one we want, fail --- this suggests that there is data we can't * use. */ - if (get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_HISTOGRAM, sortop, - NULL, - &values, &nvalues, - NULL, NULL)) + ATTSTATSSLOT_VALUES)) { - if (nvalues > 0) + if (sslot.nvalues > 0) { - tmin = datumCopy(values[0], typByVal, typLen); - tmax = datumCopy(values[nvalues - 1], typByVal, typLen); + tmin = datumCopy(sslot.values[0], typByVal, typLen); + tmax = datumCopy(sslot.values[sslot.nvalues - 1], typByVal, typLen); have_data = true; } - free_attstatsslot(vardata->atttype, values, nvalues, NULL, 0); + free_attstatsslot(&sslot); } - else if (get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + else if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_HISTOGRAM, InvalidOid, - NULL, - &values, &nvalues, - NULL, NULL)) + 0)) { - free_attstatsslot(vardata->atttype, values, nvalues, NULL, 0); + free_attstatsslot(&sslot); return false; } @@ -5199,12 +5119,9 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, * the MCVs. However, usually the MCVs will not be the extreme values, so * avoid unnecessary data copying. */ - if (get_attstatsslot(vardata->statsTuple, - vardata->atttype, vardata->atttypmod, + if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - NULL, - &values, &nvalues, - NULL, NULL)) + ATTSTATSSLOT_VALUES)) { bool tmin_is_mcv = false; bool tmax_is_mcv = false; @@ -5212,26 +5129,26 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, fmgr_info(opfuncoid, &opproc); - for (i = 0; i < nvalues; i++) + for (i = 0; i < sslot.nvalues; i++) { if (!have_data) { - tmin = tmax = values[i]; + tmin = tmax = sslot.values[i]; tmin_is_mcv = tmax_is_mcv = have_data = true; continue; } if (DatumGetBool(FunctionCall2Coll(&opproc, DEFAULT_COLLATION_OID, - values[i], tmin))) + sslot.values[i], tmin))) { - tmin = values[i]; + tmin = sslot.values[i]; tmin_is_mcv = true; } if (DatumGetBool(FunctionCall2Coll(&opproc, DEFAULT_COLLATION_OID, - tmax, values[i]))) + tmax, sslot.values[i]))) { - tmax = values[i]; + tmax = sslot.values[i]; tmax_is_mcv = true; } } @@ -5239,7 +5156,7 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, tmin = datumCopy(tmin, typByVal, typLen); if (tmax_is_mcv) tmax = datumCopy(tmax, typByVal, typLen); - free_attstatsslot(vardata->atttype, values, nvalues, NULL, 0); + free_attstatsslot(&sslot); } *min = tmin; @@ -6979,25 +6896,21 @@ btcostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (HeapTupleIsValid(vardata.statsTuple)) { Oid sortop; - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; sortop = get_opfamily_member(index->opfamily[0], index->opcintype[0], index->opcintype[0], BTLessStrategyNumber); if (OidIsValid(sortop) && - get_attstatsslot(vardata.statsTuple, InvalidOid, 0, - STATISTIC_KIND_CORRELATION, - sortop, - NULL, - NULL, NULL, - &numbers, &nnumbers)) + get_attstatsslot(&sslot, vardata.statsTuple, + STATISTIC_KIND_CORRELATION, sortop, + ATTSTATSSLOT_NUMBERS)) { double varCorrelation; - Assert(nnumbers == 1); - varCorrelation = numbers[0]; + Assert(sslot.nnumbers == 1); + varCorrelation = sslot.numbers[0]; if (index->reverse_sort[0]) varCorrelation = -varCorrelation; @@ -7007,7 +6920,7 @@ btcostestimate(PlannerInfo *root, IndexPath *path, double loop_count, else costs.indexCorrelation = varCorrelation; - free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers); + free_attstatsslot(&sslot); } } @@ -7920,25 +7833,21 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (HeapTupleIsValid(vardata.statsTuple)) { - float4 *numbers; - int nnumbers; + AttStatsSlot sslot; - if (get_attstatsslot(vardata.statsTuple, InvalidOid, 0, - STATISTIC_KIND_CORRELATION, - InvalidOid, - NULL, - NULL, NULL, - &numbers, &nnumbers)) + if (get_attstatsslot(&sslot, vardata.statsTuple, + STATISTIC_KIND_CORRELATION, InvalidOid, + ATTSTATSSLOT_NUMBERS)) { double varCorrelation = 0.0; - if (nnumbers > 0) - varCorrelation = Abs(numbers[0]); + if (sslot.nnumbers > 0) + varCorrelation = Abs(sslot.numbers[0]); if (varCorrelation > *indexCorrelation) *indexCorrelation = varCorrelation; - free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers); + free_attstatsslot(&sslot); } } diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 720e540276..b94d475505 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -32,7 +32,6 @@ #include "catalog/pg_proc.h" #include "catalog/pg_range.h" #include "catalog/pg_statistic.h" -#include "catalog/pg_statistic_ext.h" #include "catalog/pg_transform.h" #include "catalog/pg_type.h" #include "miscadmin.h" @@ -2865,35 +2864,39 @@ get_attavgwidth(Oid relid, AttrNumber attnum) * that have been provided by a stats hook and didn't really come from * pg_statistic. * + * sslot: pointer to output area (typically, a local variable in the caller). * statstuple: pg_statistic tuple to be examined. - * atttype: type OID of slot's stavalues (can be InvalidOid if values == NULL). - * atttypmod: typmod of slot's stavalues (can be 0 if values == NULL). * reqkind: STAKIND code for desired statistics slot kind. * reqop: STAOP value wanted, or InvalidOid if don't care. - * actualop: if not NULL, *actualop receives the actual STAOP value. - * values, nvalues: if not NULL, the slot's stavalues are extracted. - * numbers, nnumbers: if not NULL, the slot's stanumbers are extracted. + * flags: bitmask of ATTSTATSSLOT_VALUES and/or ATTSTATSSLOT_NUMBERS. * - * If assigned, values and numbers are set to point to palloc'd arrays. - * If the stavalues datatype is pass-by-reference, the values referenced by - * the values array are themselves palloc'd. The palloc'd stuff can be - * freed by calling free_attstatsslot. + * If a matching slot is found, TRUE is returned, and *sslot is filled thus: + * staop: receives the actual STAOP value. + * valuetype: receives actual datatype of the elements of stavalues. + * values: receives pointer to an array of the slot's stavalues. + * nvalues: receives number of stavalues. + * numbers: receives pointer to an array of the slot's stanumbers (as float4). + * nnumbers: receives number of stanumbers. * - * Note: at present, atttype/atttypmod aren't actually used here at all. - * But the caller must have the correct (or at least binary-compatible) - * type ID to pass to free_attstatsslot later. + * valuetype/values/nvalues are InvalidOid/NULL/0 if ATTSTATSSLOT_VALUES + * wasn't specified. Likewise, numbers/nnumbers are NULL/0 if + * ATTSTATSSLOT_NUMBERS wasn't specified. + * + * If no matching slot is found, FALSE is returned, and *sslot is zeroed. + * + * The data referred to by the fields of sslot is locally palloc'd and + * is independent of the original pg_statistic tuple. When the caller + * is done with it, call free_attstatsslot to release the palloc'd data. + * + * If it's desirable to call free_attstatsslot when get_attstatsslot might + * not have been called, memset'ing sslot to zeroes will allow that. */ bool -get_attstatsslot(HeapTuple statstuple, - Oid atttype, int32 atttypmod, - int reqkind, Oid reqop, - Oid *actualop, - Datum **values, int *nvalues, - float4 **numbers, int *nnumbers) +get_attstatsslot(AttStatsSlot *sslot, HeapTuple statstuple, + int reqkind, Oid reqop, int flags) { Form_pg_statistic stats = (Form_pg_statistic) GETSTRUCT(statstuple); - int i, - j; + int i; Datum val; bool isnull; ArrayType *statarray; @@ -2902,6 +2905,9 @@ get_attstatsslot(HeapTuple statstuple, HeapTuple typeTuple; Form_pg_type typeForm; + /* initialize *sslot properly */ + memset(sslot, 0, sizeof(AttStatsSlot)); + for (i = 0; i < STATISTIC_NUM_SLOTS; i++) { if ((&stats->stakind1)[i] == reqkind && @@ -2911,26 +2917,29 @@ get_attstatsslot(HeapTuple statstuple, if (i >= STATISTIC_NUM_SLOTS) return false; /* not there */ - if (actualop) - *actualop = (&stats->staop1)[i]; + sslot->staop = (&stats->staop1)[i]; - if (values) + if (flags & ATTSTATSSLOT_VALUES) { val = SysCacheGetAttr(STATRELATTINH, statstuple, Anum_pg_statistic_stavalues1 + i, &isnull); if (isnull) elog(ERROR, "stavalues is null"); - statarray = DatumGetArrayTypeP(val); /* - * Need to get info about the array element type. We look at the - * actual element type embedded in the array, which might be only - * binary-compatible with the passed-in atttype. The info we extract - * here should be the same either way, but deconstruct_array is picky - * about having an exact type OID match. + * Detoast the array if needed, and in any case make a copy that's + * under control of this AttStatsSlot. */ - arrayelemtype = ARR_ELEMTYPE(statarray); + statarray = DatumGetArrayTypePCopy(val); + + /* + * Extract the actual array element type, and pass it back in case the + * caller needs it. + */ + sslot->valuetype = arrayelemtype = ARR_ELEMTYPE(statarray); + + /* Need info about element type */ typeTuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(arrayelemtype)); if (!HeapTupleIsValid(typeTuple)) elog(ERROR, "cache lookup failed for type %u", arrayelemtype); @@ -2942,40 +2951,35 @@ get_attstatsslot(HeapTuple statstuple, typeForm->typlen, typeForm->typbyval, typeForm->typalign, - values, NULL, nvalues); + &sslot->values, NULL, &sslot->nvalues); /* * If the element type is pass-by-reference, we now have a bunch of - * Datums that are pointers into the syscache value. Copy them to - * avoid problems if syscache decides to drop the entry. + * Datums that are pointers into the statarray, so we need to keep + * that until free_attstatsslot. Otherwise, all the useful info is in + * sslot->values[], so we can free the array object immediately. */ if (!typeForm->typbyval) - { - for (j = 0; j < *nvalues; j++) - { - (*values)[j] = datumCopy((*values)[j], - typeForm->typbyval, - typeForm->typlen); - } - } + sslot->values_arr = statarray; + else + pfree(statarray); ReleaseSysCache(typeTuple); - - /* - * Free statarray if it's a detoasted copy. - */ - if ((Pointer) statarray != DatumGetPointer(val)) - pfree(statarray); } - if (numbers) + if (flags & ATTSTATSSLOT_NUMBERS) { val = SysCacheGetAttr(STATRELATTINH, statstuple, Anum_pg_statistic_stanumbers1 + i, &isnull); if (isnull) elog(ERROR, "stanumbers is null"); - statarray = DatumGetArrayTypeP(val); + + /* + * Detoast the array if needed, and in any case make a copy that's + * under control of this AttStatsSlot. + */ + statarray = DatumGetArrayTypePCopy(val); /* * We expect the array to be a 1-D float4 array; verify that. We don't @@ -2987,15 +2991,13 @@ get_attstatsslot(HeapTuple statstuple, ARR_HASNULL(statarray) || ARR_ELEMTYPE(statarray) != FLOAT4OID) elog(ERROR, "stanumbers is not a 1-D float4 array"); - *numbers = (float4 *) palloc(narrayelem * sizeof(float4)); - memcpy(*numbers, ARR_DATA_PTR(statarray), narrayelem * sizeof(float4)); - *nnumbers = narrayelem; - /* - * Free statarray if it's a detoasted copy. - */ - if ((Pointer) statarray != DatumGetPointer(val)) - pfree(statarray); + /* Give caller a pointer directly into the statarray */ + sslot->numbers = (float4 *) ARR_DATA_PTR(statarray); + sslot->nnumbers = narrayelem; + + /* We'll free the statarray in free_attstatsslot */ + sslot->numbers_arr = statarray; } return true; @@ -3004,28 +3006,19 @@ get_attstatsslot(HeapTuple statstuple, /* * free_attstatsslot * Free data allocated by get_attstatsslot - * - * atttype is the type of the individual values in values[]. - * It need be valid only if values != NULL. */ void -free_attstatsslot(Oid atttype, - Datum *values, int nvalues, - float4 *numbers, int nnumbers) +free_attstatsslot(AttStatsSlot *sslot) { - if (values) - { - if (!get_typbyval(atttype)) - { - int i; - - for (i = 0; i < nvalues; i++) - pfree(DatumGetPointer(values[i])); - } - pfree(values); - } - if (numbers) - pfree(numbers); + /* The values[] array was separately palloc'd by deconstruct_array */ + if (sslot->values) + pfree(sslot->values); + /* The numbers[] array points into numbers_arr, do not pfree it */ + /* Free the detoasted array objects, if any */ + if (sslot->values_arr) + pfree(sslot->values_arr); + if (sslot->numbers_arr) + pfree(sslot->numbers_arr); } /* ---------- PG_NAMESPACE CACHE ---------- */ diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 88629d99aa..a0d90e7e17 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -35,6 +35,28 @@ typedef enum IOFuncSelector IOFunc_send } IOFuncSelector; +/* Flag bits for get_attstatsslot */ +#define ATTSTATSSLOT_VALUES 0x01 +#define ATTSTATSSLOT_NUMBERS 0x02 + +/* Result struct for get_attstatsslot */ +typedef struct AttStatsSlot +{ + /* Always filled: */ + Oid staop; /* Actual staop for the found slot */ + /* Filled if ATTSTATSSLOT_VALUES is specified: */ + Oid valuetype; /* Actual datatype of the values */ + Datum *values; /* slot's "values" array, or NULL if none */ + int nvalues; /* length of values[], or 0 */ + /* Filled if ATTSTATSSLOT_NUMBERS is specified: */ + float4 *numbers; /* slot's "numbers" array, or NULL if none */ + int nnumbers; /* length of numbers[], or 0 */ + + /* Remaining fields are private to get_attstatsslot/free_attstatsslot */ + void *values_arr; /* palloc'd values array, if any */ + void *numbers_arr; /* palloc'd numbers array, if any */ +} AttStatsSlot; + /* Hook for plugins to get control in get_attavgwidth() */ typedef int32 (*get_attavgwidth_hook_type) (Oid relid, AttrNumber attnum); extern PGDLLIMPORT get_attavgwidth_hook_type get_attavgwidth_hook; @@ -148,15 +170,9 @@ extern Oid getBaseType(Oid typid); extern Oid getBaseTypeAndTypmod(Oid typid, int32 *typmod); extern int32 get_typavgwidth(Oid typid, int32 typmod); extern int32 get_attavgwidth(Oid relid, AttrNumber attnum); -extern bool get_attstatsslot(HeapTuple statstuple, - Oid atttype, int32 atttypmod, - int reqkind, Oid reqop, - Oid *actualop, - Datum **values, int *nvalues, - float4 **numbers, int *nnumbers); -extern void free_attstatsslot(Oid atttype, - Datum *values, int nvalues, - float4 *numbers, int nnumbers); +extern bool get_attstatsslot(AttStatsSlot *sslot, HeapTuple statstuple, + int reqkind, Oid reqop, int flags); +extern void free_attstatsslot(AttStatsSlot *sslot); extern char *get_namespace_name(Oid nspid); extern char *get_namespace_name_or_temp(Oid nspid); extern Oid get_range_subtype(Oid rangeOid); diff --git a/src/include/utils/selfuncs.h b/src/include/utils/selfuncs.h index b7617ddd8c..958bdba455 100644 --- a/src/include/utils/selfuncs.h +++ b/src/include/utils/selfuncs.h @@ -72,8 +72,8 @@ typedef struct VariableStatData /* NB: if statsTuple!=NULL, it must be freed when caller is done */ void (*freefunc) (HeapTuple tuple); /* how to free statsTuple */ Oid vartype; /* exposed type of expression */ - Oid atttype; /* type to pass to get_attstatsslot */ - int32 atttypmod; /* typmod to pass to get_attstatsslot */ + Oid atttype; /* actual type (after stripping relabel) */ + int32 atttypmod; /* actual typmod (after stripping relabel) */ bool isunique; /* matches unique index or DISTINCT clause */ bool acl_ok; /* result of ACL check on table or column */ } VariableStatData;