public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 02/15] Build inherited extended stats on partitioned tables
6+ messages / 3 participants
[nested] [flat]
* [PATCH v3 2/6] Build inherited extended stats on partitioned tables
@ 2021-09-25 21:01 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 6+ 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 4928702aec..acc994e1e8 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 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 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
--mR8QP4gmHujQHb1c
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0003-Add-stxdinherit-build-inherited-extended-stats-on.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH 02/15] Build inherited extended stats on partitioned tables
@ 2021-09-25 21:01 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
* [PATCH v2 2/5] Build inherited extended stats on partitioned tables
@ 2021-09-25 21:01 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
* [PATCH 2/5] Build inherited extended stats on partitioned tables
@ 2021-09-25 21:01 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 6+ 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 eea38a5bc7..b40ad9da2b 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 7533574fdc..d782605953 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
--Fw8vdPO5iEPGjqL+
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] 6+ messages in thread
* Re: Virtual generated columns
@ 2024-07-01 10:59 Peter Eisentraut <[email protected]>
2024-07-22 08:01 ` Re: Virtual generated columns jian he <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2024-07-01 10:59 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers
On 28.06.24 02:00, jian he wrote:
> the test structure you made ( generated_stored.sql,
> generated_virtual.sq) looks ok to me.
> but do we need to reset the search_path at the end of
> generated_stored.sql, generated_virtual.sql?
No, the session ends at the end of the test file, so we don't need to
reset session state.
> + /*
> + * TODO: This could be done, but it would need a different implementation:
> + * no rewriting, but still need to recheck any constraints.
> + */
> + if (attTup->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("ALTER TABLE / SET EXPRESSION is not supported for virtual
> generated columns"),
> + errdetail("Column \"%s\" of relation \"%s\" is a virtual generated column.",
> + colName, RelationGetRelationName(rel))));
>
> minor typo, should be
> + errmsg("ALTER TABLE SET EXPRESSION is not supported for virtual
> generated columns"),
This style "ALTER TABLE / something else" is also used for other error
messages related to ALTER TABLE subcommands, so I am using the same here.
> insert/update/delete/merge returning have problems:
> CREATE TABLE t2 (
> a int ,
> b int GENERATED ALWAYS AS (a * 2),
> d int default 22);
> insert into t2(a) select g from generate_series(1,10) g;
>
> insert into t2 select 100 returning *, (select t2.b), t2.b = t2.a * 2;
> update t2 set a = 12 returning *, (select t2.b), t2.b = t2.a * 2;
> update t2 set a = 12 returning *, (select (select t2.b)), t2.b = t2.a * 2;
> delete from t2 where t2.b = t2.a * 2 returning *, 1,((select t2.b));
>
> currently all these query, error message is "unexpected virtual
> generated column reference"
> we expect above these query work?
Yes, this is a bug. I'm looking into it.
> issue with merge:
> CREATE TABLE t0 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (a * 2) VIRTUAL);
> insert into t0(a) select g from generate_series(1,10) g;
> MERGE INTO t0 t USING t0 AS s ON 2 * t.a = s.b WHEN MATCHED THEN
> DELETE returning *;
>
> the above query returns zero rows, but for stored generated columns it
> will return 10 rows.
>
> in transformMergeStmt(ParseState *pstate, MergeStmt *stmt)
> add
> `qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;`
> before
> `assign_query_collations(pstate, qry);`
> solve the problem.
Good catch. Will fix.
Thanks for this review. I will work on fixing the issues above and come
back with a new patch set.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Virtual generated columns
2024-07-01 10:59 Re: Virtual generated columns Peter Eisentraut <[email protected]>
@ 2024-07-22 08:01 ` jian he <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: jian he @ 2024-07-22 08:01 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
statistic related bug.
borrow examples from
https://www.postgresql.org/docs/current/sql-createstatistics.html
CREATE TABLE t3 (a timestamp PRIMARY KEY, b timestamp GENERATED
ALWAYS AS (a) VIRTUAL);
CREATE STATISTICS s3 (ndistinct) ON b FROM t3;
INSERT INTO t3(a) SELECT i FROM generate_series('2020-01-01'::timestamp,
'2020-12-31'::timestamp,
'1 minute'::interval) s(i);
ANALYZE t3;
CREATE STATISTICS s3 (ndistinct) ON date_trunc('month', a),
date_trunc('day', b) FROM t3;
ANALYZE t3;
ERROR: unexpected virtual generated column reference
--this is allowed
CREATE STATISTICS s5 ON (b + interval '1 day') FROM t3;
--this is not allowed. seems inconsistent?
CREATE STATISTICS s6 ON (b ) FROM t3;
in CreateStatistics(CreateStatsStmt *stmt)
we have
if (selem->name)
{
if (attForm->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("statistics creation on virtual
generated columns is not supported")));
}
else if (IsA(selem->expr, Var)) /* column reference in parens */
{
if (get_attgenerated(relid, var->varattno) ==
ATTRIBUTE_GENERATED_VIRTUAL)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("statistics creation on virtual
generated columns is not supported")));
}
else /* expression */
{
...
}
you didn't make sure the last "else" branch is not related to virtual
generated columns
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-07-22 08:01 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-09-25 21:01 [PATCH v3 2/6] Build inherited extended stats on partitioned tables Tomas Vondra <[email protected]>
2021-09-25 21:01 [PATCH 02/15] Build inherited extended stats on partitioned tables Tomas Vondra <[email protected]>
2021-09-25 21:01 [PATCH v2 2/5] Build inherited extended stats on partitioned tables Tomas Vondra <[email protected]>
2021-09-25 21:01 [PATCH 2/5] Build inherited extended stats on partitioned tables Tomas Vondra <[email protected]>
2024-07-01 10:59 Re: Virtual generated columns Peter Eisentraut <[email protected]>
2024-07-22 08:01 ` Re: Virtual generated columns jian he <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox