public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tomas Vondra <[email protected]>
Subject: [PATCH v2 2/5] Build inherited extended stats on partitioned tables
Date: Sat, 25 Sep 2021 23:01:21 +0200

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"



view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [PATCH v2 2/5] Build inherited extended stats on partitioned tables
  In-Reply-To: <no-message-id-1860870@localhost>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox