Skip to content

Commit 9d1bcf5

Browse files
tvondratvondra
committed
Build inherited extended stats on partitioned tables
Commit 859b300 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 859b300 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 859b300), 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 859b300). Author: Justin Pryzby Reported-by: Justin Pryzby Backpatch-through: 10 Discussion: https://postgr.es/m/20210923212624.GI831%40telsasoft.com
1 parent 76569ad commit 9d1bcf5

File tree

6 files changed

+70
-15
lines changed

6 files changed

+70
-15
lines changed

‎src/backend/commands/analyze.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
513513
{
514514
MemoryContext col_context,
515515
old_context;
516+
bool build_ext_stats;
516517

517518
col_context = AllocSetContextCreate(anl_context,
518519
"Analyze Column",
@@ -573,13 +574,28 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
573574
thisdata->attr_cnt, thisdata->vacattrstats);
574575
}
575576

577+
/*
578+
* Should we build extended statistics for this relation?
579+
*
580+
* The extended statistics catalog does not include an inheritance
581+
* flag, so we can't store statistics built both with and without
582+
* data from child relations. We can store just one set of statistics
583+
* per relation. For plain relations that's fine, but for inheritance
584+
* trees we have to pick whether to store statistics for just the
585+
* one relation or the whole tree. For plain inheritance we store
586+
* the (!inh) version, mostly for backwards compatibility reasons.
587+
* For partitioned tables that's pointless (the non-leaf tables are
588+
* always empty), so we store stats representing the whole tree.
589+
*/
590+
build_ext_stats = (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) ? inh : (!inh);
591+
576592
/*
577593
* Build extended statistics (if there are any).
578594
*
579595
* For now we only build extended statistics on individual relations,
580596
* not for relations representing inheritance trees.
581597
*/
582-
if (!inh)
598+
if (build_ext_stats)
583599
BuildRelationExtStatistics(onerel, totalrows, numrows, rows,
584600
attr_cnt, vacattrstats);
585601
}

‎src/backend/statistics/dependencies.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -967,10 +967,13 @@ dependencies_clauselist_selectivity(PlannerInfo *root,
967967
RangeTblEntry *rte = planner_rt_fetch(rel->relid, root);
968968

969969
/*
970-
* When dealing with inheritance trees, ignore extended stats (which were
971-
* built without data from child rels, and thus do not represent them).
970+
* When dealing with regular inheritance trees, ignore extended stats
971+
* (which were built without data from child rels, and thus do not
972+
* represent them). For partitioned tables data there's no data in the
973+
* non-leaf relations, so we build stats only for the inheritance tree.
974+
* So for partitioned tables we do consider extended stats.
972975
*/
973-
if (rte->inh)
976+
if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
974977
return 1.0;
975978

976979
/* check if there's any stats that might be useful for us. */

‎src/backend/statistics/extended_stats.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,10 +1079,13 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
10791079
RangeTblEntry *rte = planner_rt_fetch(rel->relid, root);
10801080

10811081
/*
1082-
* When dealing with inheritance trees, ignore extended stats (which were
1083-
* built without data from child rels, and thus do not represent them).
1082+
* When dealing with regular inheritance trees, ignore extended stats
1083+
* (which were built without data from child rels, and thus do not
1084+
* represent them). For partitioned tables data there's no data in the
1085+
* non-leaf relations, so we build stats only for the inheritance tree.
1086+
* So for partitioned tables we do consider extended stats.
10841087
*/
1085-
if (rte->inh)
1088+
if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
10861089
return 1.0;
10871090

10881091
/* check if there's any stats that might be useful for us. */

‎src/backend/utils/adt/selfuncs.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3641,19 +3641,23 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel,
36413641
Oid statOid = InvalidOid;
36423642
MVNDistinct *stats;
36433643
Bitmapset *matched = NULL;
3644-
RangeTblEntry *rte = planner_rt_fetch(rel->relid, root);
3645-
3646-
/*
3647-
* When dealing with inheritance trees, ignore extended stats (which were
3648-
* built without data from child rels, and thus do not represent them).
3649-
*/
3650-
if (rte->inh)
3651-
return false;
3644+
RangeTblEntry *rte;
36523645

36533646
/* bail out immediately if the table has no extended statistics */
36543647
if (!rel->statlist)
36553648
return false;
36563649

3650+
/*
3651+
* When dealing with regular inheritance trees, ignore extended stats
3652+
* (which were built without data from child rels, and thus do not
3653+
* represent them). For partitioned tables data there's no data in the
3654+
* non-leaf relations, so we build stats only for the inheritance tree.
3655+
* So for partitioned tables we do consider extended stats.
3656+
*/
3657+
rte = planner_rt_fetch(rel->relid, root);
3658+
if (rte->inh && rte->relkind != RELKIND_PARTITIONED_TABLE)
3659+
return false;
3660+
36573661
/* Determine the attnums we're looking for */
36583662
foreach(lc, *varinfos)
36593663
{

‎src/test/regress/expected/stats_ext.out

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,25 @@ SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b
165165
(1 row)
166166

167167
DROP TABLE stxdinh, stxdinh1, stxdinh2;
168+
-- Ensure inherited stats ARE applied to inherited query in partitioned table
169+
CREATE TABLE stxdinp(i int, a int, b int) PARTITION BY RANGE (i);
170+
CREATE TABLE stxdinp1 PARTITION OF stxdinp FOR VALUES FROM (1) TO (100);
171+
INSERT INTO stxdinp SELECT 1, a/100, a/100 FROM generate_series(1, 999) a;
172+
CREATE STATISTICS stxdinp ON a, b FROM stxdinp;
173+
VACUUM ANALYZE stxdinp; -- partitions are processed recursively
174+
SELECT 1 FROM pg_statistic_ext WHERE stxrelid = 'stxdinp'::regclass;
175+
?column?
176+
----------
177+
1
178+
(1 row)
179+
180+
SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinp GROUP BY 1, 2');
181+
estimated | actual
182+
-----------+--------
183+
10 | 10
184+
(1 row)
185+
186+
DROP TABLE stxdinp;
168187
-- Verify supported object types for extended statistics
169188
CREATE schema tststats;
170189
CREATE TABLE tststats.t (a int, b int, c text);

‎src/test/regress/sql/stats_ext.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* GROUP BY 1, 2');
114114
SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinh* WHERE a = 0 AND b = 0');
115115
DROP TABLE stxdinh, stxdinh1, stxdinh2;
116116

117+
-- Ensure inherited stats ARE applied to inherited query in partitioned table
118+
CREATE TABLE stxdinp(i int, a int, b int) PARTITION BY RANGE (i);
119+
CREATE TABLE stxdinp1 PARTITION OF stxdinp FOR VALUES FROM (1) TO (100);
120+
INSERT INTO stxdinp SELECT 1, a/100, a/100 FROM generate_series(1, 999) a;
121+
CREATE STATISTICS stxdinp ON a, b FROM stxdinp;
122+
VACUUM ANALYZE stxdinp; -- partitions are processed recursively
123+
SELECT 1 FROM pg_statistic_ext WHERE stxrelid = 'stxdinp'::regclass;
124+
SELECT * FROM check_estimated_rows('SELECT a, b FROM stxdinp GROUP BY 1, 2');
125+
DROP TABLE stxdinp;
126+
117127
-- Verify supported object types for extended statistics
118128
CREATE schema tststats;
119129

0 commit comments

Comments
 (0)