From 7adbe186f7fd7e036d2cab7d8ed8ee99354ec219 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 1 Oct 2021 14:59:35 -0400 Subject: [PATCH] Avoid believing incomplete MCV-only stats in get_variable_range(). get_variable_range() would incautiously believe that statistics containing only an MCV list are sufficient to derive a range estimate. That's okay for an enum-like column that contains only MCVs, but otherwise the estimate could be pretty bad. Make it report that the range is indeterminate unless the MCVs plus nullfrac account for the whole table. I don't think this needs a dedicated test case, since a quick code coverage check verifies that the existing regression tests traverse all the alternatives. There is room to doubt that a future-proof test case could be built anyway, given that the submitted example accidentally doesn't fail before v11. Per bug #17207 from Simon Perepelitsa. Back-patch to v10. In principle this has been broken all along, but I'm hesitant to make such changes in 9.6, since if anyone is unhappy with 9.6.24's behavior there will be no second chance to fix it. Discussion: https://postgr.es/m/17207-5265aefa79e333b4@postgresql.org --- src/backend/utils/adt/selfuncs.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index c2aeb4b947..81602674fd 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -5494,15 +5494,35 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, /* * If we have most-common-values info, look for extreme MCVs. This is * needed even if we also have a histogram, since the histogram excludes - * the MCVs. + * the MCVs. However, if we *only* have MCVs and no histogram, we should + * be pretty wary of deciding that that is a full representation of the + * data. Proceed only if the MCVs represent the whole table (to within + * roundoff error). */ if (get_attstatsslot(&sslot, vardata->statsTuple, STATISTIC_KIND_MCV, InvalidOid, - ATTSTATSSLOT_VALUES)) + have_data ? ATTSTATSSLOT_VALUES : + (ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS))) { - get_stats_slot_range(&sslot, opfuncoid, &opproc, - collation, typLen, typByVal, - &tmin, &tmax, &have_data); + bool use_mcvs = have_data; + + if (!have_data) + { + double sumcommon = 0.0; + double nullfrac; + int i; + + for (i = 0; i < sslot.nnumbers; i++) + sumcommon += sslot.numbers[i]; + nullfrac = ((Form_pg_statistic) GETSTRUCT(vardata->statsTuple))->stanullfrac; + if (sumcommon + nullfrac > 0.99999) + use_mcvs = true; + } + + if (use_mcvs) + get_stats_slot_range(&sslot, opfuncoid, &opproc, + collation, typLen, typByVal, + &tmin, &tmax, &have_data); free_attstatsslot(&sslot); }