Build inherited extended stats on partitioned tables

Commit 859b3003de disabled building of extended stats for inheritance
trees, to prevent updating the same catalog row twice. While that
resolved the issue, it also means there are no extended stats for
declaratively partitioned tables, because there are no data in the
non-leaf relations.

That also means declaratively partitioned tables were not affected by
the issue 859b3003de addressed, which means this is a regression
affecting queries that calculate estimates for the whole inheritance
tree as a whole (which includes e.g. GROUP BY queries).

But because partitioned tables are empty, we can invert the condition
and build statistics only for the case with inheritance, without losing
anything. And we can consider them when calculating estimates.

It may be necessary to run ANALYZE on partitioned tables, to collect
proper statistics. For declarative partitioning there should no prior
statistics, and it might take time before autoanalyze is triggered. For
tables partitioned by inheritance the statistics may include data from
child relations (if built 859b3003de), contradicting the current code.

Report and patch by Justin Pryzby, minor fixes and cleanup by me.
Backpatch all the way back to PostgreSQL 10, where extended statistics
were introduced (same as 859b3003de).

Author: Justin Pryzby
Reported-by: Justin Pryzby
Backpatch-through: 10
Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 4928702aec0f61263d7203f6d3cf6a5ca5f4c67b..8248c0c050e1796ed7d81e22788d1284ba300d6e 100644 (file)
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -548,6 +548,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
    {
        MemoryContext col_context,
                    old_context;
+       bool        build_ext_stats;
 
        pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
                                     PROGRESS_ANALYZE_PHASE_COMPUTE_STATS);
@@ -611,13 +612,28 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
                            thisdata->attr_cnt, thisdata->vacattrstats);
        }
 
+       /*
+        * Should we build extended statistics for this relation?
+        *
+        * The extended statistics catalog does not include an inheritance
+        * flag, so we can't store statistics built both with and without
+        * data from child relations. We can store just one set of statistics
+        * per relation. For plain relations that's fine, but for inheritance
+        * trees we have to pick whether to store statistics for just the
+        * one relation or the whole tree. For plain inheritance we store
+        * the (!inh) version, mostly for backwards compatibility reasons.
+        * For partitioned tables that's pointless (the non-leaf tables are
+        * always empty), so we store stats representing the whole tree.
+        */
+       build_ext_stats = (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) ? inh : (!inh);
+
        /*
         * Build extended statistics (if there are any).
         *
         * For now we only build extended statistics on individual relations,
         * not for relations representing inheritance trees.
         */
-       if (!inh)
+       if (build_ext_stats)
            BuildRelationExtStatistics(onerel, totalrows, numrows, rows,
                                       attr_cnt, vacattrstats);
    }

diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c
index a41bace75abe18986e63dc620aae0f1a9d2fd69d..a9187ddeb1516dcd569e09fa8a78966ed72070b5 100644 (file)
--- a/src/backend/statistics/dependencies.c
+++ b/src/backend/statistics/dependencies.c
@@ -1418,10 +1418,13 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
    int         unique_exprs_cnt;
 
    /*
-    * When dealing with inheritance trees, ignore extended stats (which were
-    * built without data from child rels, and thus do not represent them).
+    * When dealing with regular inheritance trees, ignore extended stats
+    * (which were built without data from child rels, and thus do not
+    * represent them). For partitioned tables data there's no data in the
+    * non-leaf relations, so we build stats only for the inheritance tree.
+    * So for partitioned tables we do consider extended stats.
     */
-   if (rte->inh)
+   if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
        return 1.0;
 
    /* check if there's any stats that might be useful for us. */

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 366ae334f8b65a39e28bac77cc232513e95bb108..6761c57f0ace20f0150515ef140ad708d5b6c71f 100644 (file)
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1698,10 +1698,13 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
    RangeTblEntry *rte = planner_rt_fetch(rel->relid, root);
 
    /*
-    * When dealing with inheritance trees, ignore extended stats (which were
-    * built without data from child rels, and thus do not represent them).
+    * When dealing with regular inheritance trees, ignore extended stats
+    * (which were built without data from child rels, and thus do not
+    * represent them). For partitioned tables data there's no data in the
+    * non-leaf relations, so we build stats only for the inheritance tree.
+    * So for partitioned tables we do consider extended stats.
     */
-   if (rte->inh)
+   if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
        return sel;
 
    /* check if there's any stats that might be useful for us. */

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 1010d5caa86a3052666927e9acd04bda3e8d48f1..abe47dab866ad4c583b2c57c910e210f5153e46f 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -3913,19 +3913,23 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel,
    Oid         statOid = InvalidOid;
    MVNDistinct *stats;
    StatisticExtInfo *matched_info = NULL;
-   RangeTblEntry       *rte = planner_rt_fetch(rel->relid, root);
-
-   /*
-    * When dealing with inheritance trees, ignore extended stats (which were
-    * built without data from child rels, and thus do not represent them).
-    */
-   if (rte->inh)
-       return false;
+   RangeTblEntry       *rte;
 
    /* bail out immediately if the table has no extended statistics */
    if (!rel->statlist)
        return false;
 
+   /*
+    * When dealing with regular inheritance trees, ignore extended stats
+    * (which were built without data from child rels, and thus do not
+    * represent them). For partitioned tables data there's no data in the
+    * non-leaf relations, so we build stats only for the inheritance tree.
+    * So for partitioned tables we do consider extended stats.
+    */
+   rte = planner_rt_fetch(rel->relid, root);
+   if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
+       return false;
+
    /* look for the ndistinct statistics object matching the most vars */
    nmatches_vars = 0;          /* we require at least two matches */
    nmatches_exprs = 0;
@@ -5242,11 +5246,14 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                break;
 
            /*
-            * When dealing with inheritance trees, ignore extended stats (which
-            * were built without data from child rels, and so do not represent
-            * them).
+            * When dealing with regular inheritance trees, ignore extended
+            * stats (which were built without data from child rels, and thus
+            * do not represent them). For partitioned tables data there's no
+            * data in the non-leaf relations, so we build stats only for the
+            * inheritance tree. So for partitioned tables we do consider
+            * extended stats.
             */
-           if (rte->inh)
+           if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
                break;
 
            /* skip stats without per-expression stats */

diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index a8f7f5f2f1289311660c76beaa3c2970210bb985..b0548108f01d3ea21622cfd201c4886e9f7b668a 100644 (file)
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -217,6 +217,25 @@ SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b
 (1 row)
 
 DROP TABLE stxdinh, stxdinh1, stxdinh2;
+-- Ensure inherited stats ARE applied to inherited query in partitioned table
+CREATE TABLE stxdinp(i int, a int, b int) PARTITION BY RANGE (i);
+CREATE TABLE stxdinp1 PARTITION OF stxdinp FOR VALUES FROM (1) TO (100);
+INSERT INTO stxdinp SELECT 1, a/100, a/100 FROM generate_series(1, 999) a;
+CREATE STATISTICS stxdinp ON a, b FROM stxdinp;
+VACUUM ANALYZE stxdinp; -- partitions are processed recursively
+SELECT 1 FROM pg_statistic_ext WHERE stxrelid = 'stxdinp'::regclass;
+ ?column? 
+----------
+        1
+(1 row)
+
+SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinp GROUP BY 1, 2');
+ estimated | actual 
+-----------+--------
+        10 |     10
+(1 row)
+
+DROP TABLE stxdinp;
 -- basic test for statistics on expressions
 CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);
 -- expression stats may be built on a single expression column

diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 69d7b71f0eb8731fad8a58b68826333855f8b200..d2c59b0a8ad29693f2d720dec1a9e09ff17060f7 100644 (file)
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -134,6 +134,16 @@ SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* GROUP BY 1, 2');
 SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b = 0');
 DROP TABLE stxdinh, stxdinh1, stxdinh2;
 
+-- Ensure inherited stats ARE applied to inherited query in partitioned table
+CREATE TABLE stxdinp(i int, a int, b int) PARTITION BY RANGE (i);
+CREATE TABLE stxdinp1 PARTITION OF stxdinp FOR VALUES FROM (1) TO (100);
+INSERT INTO stxdinp SELECT 1, a/100, a/100 FROM generate_series(1, 999) a;
+CREATE STATISTICS stxdinp ON a, b FROM stxdinp;
+VACUUM ANALYZE stxdinp; -- partitions are processed recursively
+SELECT 1 FROM pg_statistic_ext WHERE stxrelid = 'stxdinp'::regclass;
+SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinp GROUP BY 1, 2');
+DROP TABLE stxdinp;
+
 -- basic test for statistics on expressions
 CREATE TABLE ab1 (a INTEGER, b INTEGER, c TIMESTAMP, d TIMESTAMPTZ);