agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH 02/15] Build inherited extended stats on partitioned tables 2+ messages / 1 participants [nested] [flat]
* [PATCH 02/15] Build inherited extended stats on partitioned tables @ 2021-09-25 21:01 Tomas Vondra <tomas.vondra@enterprisedb.com> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2021-09-25 21:01 UTC (permalink / raw) Since 859b3003de, ext stats on partitioned tables are not built, which is a regression. For back branches, pg_statistic_ext cannot support both inherited (FROM) and non-inherited (FROM ONLY) stats on inheritance heirarchies. But there's no issue building inherited stats for partitioned tables, which are empty, so cannot have non-inherited stats. See also: 8c5cdb7f4f6e1d6a6104cb58ce4f23453891651b https://www.postgresql.org/message-id/20210923212624.GI831%40telsasoft.com Backpatch to v10 --- src/backend/commands/analyze.c | 5 ++++- src/backend/statistics/dependencies.c | 2 +- src/backend/statistics/extended_stats.c | 2 +- src/backend/utils/adt/selfuncs.c | 9 ++++++--- src/test/regress/expected/stats_ext.out | 19 +++++++++++++++++++ src/test/regress/sql/stats_ext.sql | 10 ++++++++++ 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index cd77907fc7..d35325f560 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -549,6 +549,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); @@ -612,13 +613,15 @@ do_analyze_rel(Relation onerel, VacuumParams *params, thisdata->attr_cnt, thisdata->vacattrstats); } + 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 015b9e28f6..36121d5a27 100644 --- a/src/backend/statistics/dependencies.c +++ b/src/backend/statistics/dependencies.c @@ -1417,7 +1417,7 @@ dependencies_clauselist_selectivity(PlannerInfo *root, int unique_exprs_cnt; /* If it's an inheritence tree, skip statistics (which do not include child 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 b9e755f44e..5fbdbf0164 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -1697,7 +1697,7 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli RangeTblEntry *rte = root->simple_rte_array[rel->relid]; /* If it's an inheritence tree, skip statistics (which do not include child 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 a0932e39e1..b15f14e1a0 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -3916,7 +3916,7 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte = root->simple_rte_array[rel->relid]; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (rte->inh) + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) return false; /* bail out immediately if the table has no extended statistics */ @@ -5238,8 +5238,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, break; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (planner_rt_fetch(onerel->relid, root)->inh) - break; + { + RangeTblEntry *rte = planner_rt_fetch(onerel->relid, root); + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) + break; + } /* skip stats without per-expression stats */ if (info->kind != STATS_EXT_EXPRESSIONS) diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 5410f58f7f..a11fff655a 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -200,6 +200,25 @@ SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2'); (1 row) DROP TABLE stxdinh, stxdinh1; +-- 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 a196d5e2f8..01383e8f5f 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -127,6 +127,16 @@ VACUUM ANALYZE stxdinh, stxdinh1; SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2'); DROP TABLE stxdinh, stxdinh1; +-- 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); -- 2.17.0 --qmeTShRBchLQhVIG Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-Add-stxdinherit-build-inherited-extended-stats-on-in.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH v2 2/5] Build inherited extended stats on partitioned tables @ 2021-09-25 21:01 Tomas Vondra <tomas.vondra@enterprisedb.com> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2021-09-25 21:01 UTC (permalink / raw) Since 859b3003de, ext stats on partitioned tables are not built, which is a regression. For back branches, pg_statistic_ext cannot support both inherited (FROM) and non-inherited (FROM ONLY) stats on inheritence heirarchies. But there's no issue building inherited stats for partitioned tables, which are empty, so cannot have non-inherited stats. See also: 8c5cdb7f4f6e1d6a6104cb58ce4f23453891651b https://www.postgresql.org/message-id/20210923212624.GI831%40telsasoft.com Backpatch to v10 --- src/backend/commands/analyze.c | 5 ++++- src/backend/statistics/dependencies.c | 2 +- src/backend/statistics/extended_stats.c | 2 +- src/backend/utils/adt/selfuncs.c | 9 ++++++--- src/test/regress/expected/stats_ext.out | 19 +++++++++++++++++++ src/test/regress/sql/stats_ext.sql | 10 ++++++++++ 6 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 8bfb2ad958..299f4893b8 100644 --- 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,15 @@ do_analyze_rel(Relation onerel, VacuumParams *params, thisdata->attr_cnt, thisdata->vacattrstats); } + 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 b2e33329c7..0659307b02 100644 --- a/src/backend/statistics/dependencies.c +++ b/src/backend/statistics/dependencies.c @@ -1596,7 +1596,7 @@ dependencies_clauselist_selectivity(PlannerInfo *root, RangeTblEntry *rte = root->simple_rte_array[rel->relid]; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (rte->inh) + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) break; /* skip statistics that are not of the correct type */ diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 6c69e5bc96..9e518830ae 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -1747,7 +1747,7 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli RangeTblEntry *rte = root->simple_rte_array[rel->relid]; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (rte->inh) + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) break; /* find the best suited statistics object for these attnums */ diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index a0932e39e1..b15f14e1a0 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -3916,7 +3916,7 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte = root->simple_rte_array[rel->relid]; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (rte->inh) + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) return false; /* bail out immediately if the table has no extended statistics */ @@ -5238,8 +5238,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, break; /* If it's an inheritence tree, skip statistics (which do not include child stats) */ - if (planner_rt_fetch(onerel->relid, root)->inh) - break; + { + RangeTblEntry *rte = planner_rt_fetch(onerel->relid, root); + if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE) + break; + } /* skip stats without per-expression stats */ if (info->kind != STATS_EXT_EXPRESSIONS) diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 5c15e44bd6..67234b9fc2 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -199,6 +199,25 @@ SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2'); (1 row) DROP TABLE stxdinh, stxdinh1; +-- 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 610f7ed17f..2371043ca1 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -126,6 +126,16 @@ VACUUM ANALYZE stxdinh, stxdinh1; SELECT * FROM check_estimated_rows('SELECT * FROM stxdinh* GROUP BY 1,2'); DROP TABLE stxdinh, stxdinh1; +-- 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); -- 2.17.0 --CUfgB8w4ZwR/yMy5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0003-Add-stxdinherit-build-inherited-extended-stats-on.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2021-09-25 21:01 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-09-25 21:01 [PATCH 02/15] Build inherited extended stats on partitioned tables Tomas Vondra <tomas.vondra@enterprisedb.com> 2021-09-25 21:01 [PATCH v2 2/5] Build inherited extended stats on partitioned tables Tomas Vondra <tomas.vondra@enterprisedb.com>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox