Fix extended statistics with partial analyzes

Either because of a previous ALTER TABLE .. SET STATISTICS 0 or because
of being invoked with a partial column list, ANALYZE could fail to
acquire sufficient data to build extended statistics.  Previously, this
would draw an ERROR and fail to collect any statistics at all (extended
and regular).  Change things so that we raise a WARNING instead, and
remove a hint that was wrong in half the cases.

Reported by: David Rowley
Discussion: https://postgr.es/m/CAKJS1f9Kk0NF6Fg7TA=JUXsjpS9kX6NVu27pb5QDCpOYAvb-Og@mail.gmail.com

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 4b3aa7781409ae198f8900aa2715af0df0791420..64d0cc3e691e0c678a44f618cde05c6b5ced9fe6 100644 (file)
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -23,11 +23,13 @@
 #include "catalog/pg_collation.h"
 #include "catalog/pg_statistic_ext.h"
 #include "nodes/relation.h"
+#include "postmaster/autovacuum.h"
 #include "statistics/extended_stats_internal.h"
 #include "statistics/statistics.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
+#include "utils/memutils.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
 
@@ -38,6 +40,8 @@
 typedef struct StatExtEntry
 {
        Oid                     statOid;        /* OID of pg_statistic_ext entry */
+       char       *schema;             /* statistics schema */
+       char       *name;               /* statistics name */
        Bitmapset  *columns;    /* attribute numbers covered by the statistics */
        List       *types;              /* 'char' list of enabled statistic kinds */
 } StatExtEntry;
@@ -45,7 +49,7 @@ typedef struct StatExtEntry
 
 static List *fetch_statentries_for_relation(Relation pg_statext, Oid relid);
 static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
-                                         int natts, VacAttrStats **vacattrstats);
+                                         int nvacatts, VacAttrStats **vacatts);
 static void statext_store(Relation pg_stext, Oid relid,
                          MVNDistinct *ndistinct, MVDependencies *dependencies,
                          VacAttrStats **stats);
@@ -66,6 +70,12 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
        Relation        pg_stext;
        ListCell   *lc;
        List       *stats;
+       MemoryContext cxt;
+       MemoryContext oldcxt;
+
+       cxt = AllocSetContextCreate(CurrentMemoryContext, "stats ext",
+                                                               ALLOCSET_DEFAULT_SIZES);
+       oldcxt = MemoryContextSwitchTo(cxt);
 
        pg_stext = heap_open(StatisticExtRelationId, RowExclusiveLock);
        stats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
@@ -78,9 +88,23 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
                VacAttrStats  **stats;
                ListCell           *lc2;
 
-               /* filter only the interesting vacattrstats records */
+               /*
+                * Check if we can build these stats based on the column analyzed.
+                * If not, report this fact (except in autovacuum) and move on.
+                */
                stats = lookup_var_attr_stats(onerel, stat->columns,
                                                                          natts, vacattrstats);
+               if (!stats && !IsAutoVacuumWorkerProcess())
+               {
+                       ereport(WARNING,
+                                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                        errmsg("extended statistics \"%s.%s\" could not be collected for relation %s.%s",
+                                                       stat->schema, stat->name,
+                                                       get_namespace_name(onerel->rd_rel->relnamespace),
+                                                       RelationGetRelationName(onerel)),
+                                        errtable(onerel)));
+                       continue;
+               }
 
                /* check allowed number of dimensions */
                Assert(bms_num_members(stat->columns) >= 2 &&
@@ -104,6 +128,9 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
        }
 
        heap_close(pg_stext, RowExclusiveLock);
+
+       MemoryContextSwitchTo(oldcxt);
+       MemoryContextDelete(cxt);
 }
 
 /*
@@ -168,6 +195,8 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
                entry = palloc0(sizeof(StatExtEntry));
                entry->statOid = HeapTupleGetOid(htup);
                staForm = (Form_pg_statistic_ext) GETSTRUCT(htup);
+               entry->schema = get_namespace_name(staForm->stanamespace);
+               entry->name = pstrdup(NameStr(staForm->staname));
                for (i = 0; i < staForm->stakeys.dim1; i++)
                {
                        entry->columns = bms_add_member(entry->columns,
@@ -200,18 +229,19 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
 }
 
 /*
- * Using 'vacattrstats' of size 'natts' as input data, return a newly built
- * VacAttrStats array which includes only the items corresponding to attributes
- * indicated by 'attrs'.
+ * Using 'vacatts' of size 'nvacatts' as input data, return a newly built
+ * VacAttrStats array which includes only the items corresponding to
+ * attributes indicated by 'stakeys'. If we don't have all of the per column
+ * stats available to compute the extended stats, then we return NULL to indicate
+ * to the caller that the stats should not be built.
  */
 static VacAttrStats **
-lookup_var_attr_stats(Relation rel, Bitmapset *attrs, int natts,
-                                         VacAttrStats **vacattrstats)
+lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
+                                         int nvacatts, VacAttrStats **vacatts)
 {
        int                     i = 0;
        int                     x = -1;
        VacAttrStats **stats;
-       Bitmapset  *matched = NULL;
 
        stats = (VacAttrStats **)
                palloc(bms_num_members(attrs) * sizeof(VacAttrStats *));
@@ -222,39 +252,34 @@ lookup_var_attr_stats(Relation rel, Bitmapset *attrs, int natts,
                int             j;
 
                stats[i] = NULL;
-               for (j = 0; j < natts; j++)
+               for (j = 0; j < nvacatts; j++)
                {
-                       if (x == vacattrstats[j]->tupattnum)
+                       if (x == vacatts[j]->tupattnum)
                        {
-                               stats[i] = vacattrstats[j];
+                               stats[i] = vacatts[j];
                                break;
                        }
                }
 
                if (!stats[i])
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                                        errmsg("extended statistics could not be collected for column \"%s\" of relation %s.%s",
-                                                       NameStr(RelationGetDescr(rel)->attrs[x - 1]->attname),
-                                                       get_namespace_name(rel->rd_rel->relnamespace),
-                                                       RelationGetRelationName(rel)),
-                                        errhint("Consider ALTER TABLE \"%s\".\"%s\" ALTER \"%s\" SET STATISTICS -1",
-                                                        get_namespace_name(rel->rd_rel->relnamespace),
-                                                        RelationGetRelationName(rel),
-                                                        NameStr(RelationGetDescr(rel)->attrs[x - 1]->attname))));
+               {
+                       /*
+                        * Looks like stats were not gathered for one of the columns
+                        * required. We'll be unable to build the extended stats without
+                        * this column.
+                        */
+                       pfree(stats);
+                       return NULL;
+               }
 
-               /*
-                * Check that we found a non-dropped column and that the attnum
-                * matches.
-                */
+                /*
+                 * Sanity check that the column is not dropped - stats should have
+                 * been removed in this case.
+                 */
                Assert(!stats[i]->attr->attisdropped);
-               matched = bms_add_member(matched, stats[i]->tupattnum);
 
                i++;
        }
-       if (bms_subset_compare(matched, attrs) != BMS_EQUAL)
-               elog(ERROR, "could not find all attributes in attribute stats array");
-       bms_free(matched);
 
        return stats;
 }

diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index b43208d7d88f4f388013d251f149ec345998a292..6a3345548a21d73c6c103a45bd5fc7296b140acf 100644 (file)
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -40,9 +40,11 @@ ALTER TABLE ab1 ALTER a SET STATISTICS 0;
 INSERT INTO ab1 SELECT a, a%23 FROM generate_series(1, 1000) a;
 CREATE STATISTICS ab1_a_b_stats ON (a, b) FROM ab1;
 ANALYZE ab1;
-ERROR:  extended statistics could not be collected for column "a" of relation public.ab1
-HINT:  Consider ALTER TABLE "public"."ab1" ALTER "a" SET STATISTICS -1
+WARNING:  extended statistics "public.ab1_a_b_stats" could not be collected for relation public.ab1
 ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- partial analyze doesn't build stats either
+ANALYZE ab1 (a);
+WARNING:  extended statistics "public.ab1_a_b_stats" could not be collected for relation public.ab1
 ANALYZE ab1;
 DROP TABLE ab1;
 -- n-distinct tests

diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 1b0018dcea42ea9e938345b6f3388d8dac5f882b..ebb6a78383e573c942f0dd6e11dc71c9348af6b7 100644 (file)
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -35,6 +35,8 @@ INSERT INTO ab1 SELECT a, a%23 FROM generate_series(1, 1000) a;
 CREATE STATISTICS ab1_a_b_stats ON (a, b) FROM ab1;
 ANALYZE ab1;
 ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- partial analyze doesn't build stats either
+ANALYZE ab1 (a);
 ANALYZE ab1;
 DROP TABLE ab1;