Avoid believing incomplete MCV-only stats in get_variable_range().
authorTom Lane <[email protected]>
Fri, 1 Oct 2021 18:59:35 +0000 (14:59 -0400)
committerTom Lane <[email protected]>
Fri, 1 Oct 2021 18:59:35 +0000 (14:59 -0400)
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

index 60549d76b7ae9fcf5d1b8d523e11c84826479cfd..10895fb2876f8312527b6a4f7c9be08bcb7fdc38 100644 (file)
@@ -5841,15 +5841,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);
    }