From acfde7c5837dd3c2a40ff9cc639ba5d914e8e8ea Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sat, 15 Jan 2022 02:15:23 +0100 Subject: [PATCH] Ignore extended statistics for inheritance trees Since commit 859b3003de we only build extended statistics for individual relations, ignoring the child relations. This resolved the issue with updating catalog tuple twice, but we still tried to use the statistics when calculating estimates for the whole inheritance tree. When the relations contain very distinct data, it may produce bogus estimates. This is roughly the same issue 427c6b5b9 addressed ~15 years ago, and we fix it the same way - by ignoring extended statistics when calculating estimates for the inheritance tree as a whole. We still consider extended statistics when calculating estimates for individual child relations, of course. This may result in plan changes due to different estimates, but if the old statistics were not describing the inheritance tree particularly well it's quite likely the new plans is actually better. 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 --- src/backend/statistics/dependencies.c | 9 ++++++ src/backend/statistics/extended_stats.c | 9 ++++++ src/backend/utils/adt/selfuncs.c | 8 +++++ src/test/regress/expected/stats_ext.out | 41 +++++++++++++++++++++++++ src/test/regress/sql/stats_ext.sql | 22 +++++++++++++ 5 files changed, 89 insertions(+) diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c index 38eb26f311..ae19e66f20 100644 --- a/src/backend/statistics/dependencies.c +++ b/src/backend/statistics/dependencies.c @@ -24,6 +24,7 @@ #include "nodes/pathnodes.h" #include "optimizer/clauses.h" #include "optimizer/optimizer.h" +#include "parser/parsetree.h" #include "statistics/extended_stats_internal.h" #include "statistics/statistics.h" #include "utils/bytea.h" @@ -1213,6 +1214,14 @@ dependencies_clauselist_selectivity(PlannerInfo *root, MVDependency **dependencies; int ndependencies; int i; + 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 1.0; /* check if there's any stats that might be useful for us. */ if (!has_stats_of_kind(rel->statlist, STATS_EXT_DEPENDENCIES)) diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 4c19336b29..fe3fb21145 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -29,6 +29,7 @@ #include "nodes/nodeFuncs.h" #include "optimizer/clauses.h" #include "optimizer/optimizer.h" +#include "parser/parsetree.h" #include "pgstat.h" #include "postmaster/autovacuum.h" #include "statistics/extended_stats_internal.h" @@ -1294,6 +1295,14 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli Bitmapset **list_attnums; int listidx; Selectivity sel = 1.0; + 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 1.0; /* check if there's any stats that might be useful for us. */ if (!has_stats_of_kind(rel->statlist, STATS_EXT_MCV)) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 81602674fd..b648a02fbb 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -3887,6 +3887,14 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, Oid statOid = InvalidOid; MVNDistinct *stats; Bitmapset *matched = 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; /* bail out immediately if the table has no extended statistics */ if (!rel->statlist) diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 8288ee1b93..e5b9994c24 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -161,6 +161,47 @@ CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1; ANALYZE ab1; DROP TABLE ab1 CASCADE; NOTICE: drop cascades to table ab1c +-- Tests for stats with inheritance +CREATE TABLE stxdinh(a int, b int); +CREATE TABLE stxdinh1() INHERITS(stxdinh); +CREATE TABLE stxdinh2() INHERITS(stxdinh); +INSERT INTO stxdinh SELECT mod(a,50), mod(a,100) FROM generate_series(0, 1999) a; +INSERT INTO stxdinh1 SELECT mod(a,100), mod(a,100) FROM generate_series(0, 999) a; +INSERT INTO stxdinh2 SELECT mod(a,100), mod(a,100) FROM generate_series(0, 999) a; +VACUUM ANALYZE stxdinh, stxdinh1, stxdinh2; +-- Ensure non-inherited stats are not applied to inherited query +-- Without stats object, it looks like this +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* GROUP BY 1, 2'); + estimated | actual +-----------+-------- + 400 | 150 +(1 row) + +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b = 0'); + estimated | actual +-----------+-------- + 3 | 40 +(1 row) + +CREATE STATISTICS stxdinh ON a, b FROM stxdinh; +VACUUM ANALYZE stxdinh, stxdinh1, stxdinh2; +-- Since the stats object does not include inherited stats, it should not +-- affect the estimates +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* GROUP BY 1, 2'); + estimated | actual +-----------+-------- + 400 | 150 +(1 row) + +-- Dependencies are applied at individual relations (within append), so +-- this estimate changes a bit because we improve estimates for the parent +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b = 0'); + estimated | actual +-----------+-------- + 22 | 40 +(1 row) + +DROP TABLE stxdinh, stxdinh1, stxdinh2; -- Verify supported object types for extended statistics CREATE schema tststats; CREATE TABLE tststats.t (a int, b int, c text); diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 8075172319..a547ca0d13 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -106,6 +106,28 @@ CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1; ANALYZE ab1; DROP TABLE ab1 CASCADE; +-- Tests for stats with inheritance +CREATE TABLE stxdinh(a int, b int); +CREATE TABLE stxdinh1() INHERITS(stxdinh); +CREATE TABLE stxdinh2() INHERITS(stxdinh); +INSERT INTO stxdinh SELECT mod(a,50), mod(a,100) FROM generate_series(0, 1999) a; +INSERT INTO stxdinh1 SELECT mod(a,100), mod(a,100) FROM generate_series(0, 999) a; +INSERT INTO stxdinh2 SELECT mod(a,100), mod(a,100) FROM generate_series(0, 999) a; +VACUUM ANALYZE stxdinh, stxdinh1, stxdinh2; +-- Ensure non-inherited stats are not applied to inherited query +-- Without stats object, it looks like this +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'); +CREATE STATISTICS stxdinh ON a, b FROM stxdinh; +VACUUM ANALYZE stxdinh, stxdinh1, stxdinh2; +-- Since the stats object does not include inherited stats, it should not +-- affect the estimates +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* GROUP BY 1, 2'); +-- Dependencies are applied at individual relations (within append), so +-- this estimate changes a bit because we improve estimates for the parent +SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b = 0'); +DROP TABLE stxdinh, stxdinh1, stxdinh2; + -- Verify supported object types for extended statistics CREATE schema tststats;