>
> Code fix with comment on why nobody expects a relpages -1. Test case to
> demonstrate that relpages -1 can happen, and updated doc to reflect the new
> lower bound.
>
Additional fixes, now in a patch-set:
1. Allow relpages to be set to -1 (partitioned tables with partitions have
this value after ANALYZE).
2. Turn off autovacuum on tables (where possible) if they are going to be
the target of pg_set_relation_stats().
3. Allow pg_set_relation_stats to continue past an out-of-range detection
on one attribute, rather than immediately returning false.
From c0efe3fb2adac102e186e086bf94fb29fabba28b Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huin...@gmail.com>
Date: Tue, 15 Oct 2024 19:09:59 -0400
Subject: [PATCH v2 1/3] Allow pg_set_relation_stats() to set relpages to -1.
While the default value for relpages is 0, if a partitioned table with
at least one child has been analyzed, then the partititoned table will
have a relpages value of -1. pg_set_relation_stats() should therefore be
able to set relpages to -1.
Add test case to demonstrate the behavior of ANALYZE on a partitioned
table with child table(s), and to demonstrate that
pg_set_relation_stats() now accepts -1.
---
src/backend/statistics/relation_stats.c | 8 +++--
src/test/regress/expected/stats_import.out | 38 +++++++++++++++++++++-
src/test/regress/sql/stats_import.sql | 25 ++++++++++++++
doc/src/sgml/func.sgml | 2 +-
4 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 26f15061e8..b90f70b190 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -99,11 +99,15 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
{
int32 relpages = PG_GETARG_INT32(RELPAGES_ARG);
- if (relpages < 0)
+ /*
+ * While the default value for relpages is 0, a partitioned table with at
+ * least one child partition can have a relpages of -1.
+ */
+ if (relpages < -1)
{
ereport(elevel,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("relpages cannot be < 0")));
+ errmsg("relpages cannot be < -1")));
table_close(crel, RowExclusiveLock);
return false;
}
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index cd1b80aa43..46e45a5fa3 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -135,9 +135,45 @@ SELECT
'stats_import.testview'::regclass);
ERROR: cannot modify statistics for relation "testview"
DETAIL: This operation is not supported for views.
+-- Partitioned tables with at least 1 child partition will, when analyzed,
+-- have a relpages of -1
+CREATE TABLE stats_import.part_parent ( i integer ) PARTITION BY RANGE(i);
+CREATE TABLE stats_import.part_child_1
+ PARTITION OF stats_import.part_parent
+ FOR VALUES FROM (0) TO (10);
+ANALYZE stats_import.part_parent;
+SELECT relpages
+FROM pg_class
+WHERE oid = 'stats_import.part_parent'::regclass;
+ relpages
+----------
+ -1
+(1 row)
+
+-- nothing stops us from setting it to not -1
+SELECT
+ pg_catalog.pg_set_relation_stats(
+ relation => 'stats_import.part_parent'::regclass,
+ relpages => 2::integer);
+ pg_set_relation_stats
+-----------------------
+ t
+(1 row)
+
+-- nothing stops us from setting it to -1
+SELECT
+ pg_catalog.pg_set_relation_stats(
+ relation => 'stats_import.part_parent'::regclass,
+ relpages => -1::integer);
+ pg_set_relation_stats
+-----------------------
+ t
+(1 row)
+
DROP SCHEMA stats_import CASCADE;
-NOTICE: drop cascades to 4 other objects
+NOTICE: drop cascades to 5 other objects
DETAIL: drop cascades to type stats_import.complex_type
drop cascades to table stats_import.test
drop cascades to sequence stats_import.testseq
drop cascades to view stats_import.testview
+drop cascades to table stats_import.part_parent
diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index 3e9f6d9124..ad129b1ab9 100644
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -95,4 +95,29 @@ SELECT
pg_catalog.pg_clear_relation_stats(
'stats_import.testview'::regclass);
+-- Partitioned tables with at least 1 child partition will, when analyzed,
+-- have a relpages of -1
+CREATE TABLE stats_import.part_parent ( i integer ) PARTITION BY RANGE(i);
+CREATE TABLE stats_import.part_child_1
+ PARTITION OF stats_import.part_parent
+ FOR VALUES FROM (0) TO (10);
+
+ANALYZE stats_import.part_parent;
+
+SELECT relpages
+FROM pg_class
+WHERE oid = 'stats_import.part_parent'::regclass;
+
+-- nothing stops us from setting it to not -1
+SELECT
+ pg_catalog.pg_set_relation_stats(
+ relation => 'stats_import.part_parent'::regclass,
+ relpages => 2::integer);
+
+-- nothing stops us from setting it to -1
+SELECT
+ pg_catalog.pg_set_relation_stats(
+ relation => 'stats_import.part_parent'::regclass,
+ relpages => -1::integer);
+
DROP SCHEMA stats_import CASCADE;
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index f8a0d76d12..ad663c94d7 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -30197,7 +30197,7 @@ DETAIL: Make sure pg_wal_replay_wait() isn't called within a transaction with a
</para>
<para>
The value of <structfield>relpages</structfield> must be greater than
- or equal to <literal>0</literal>,
+ or equal to <literal>-1</literal>,
<structfield>reltuples</structfield> must be greater than or equal to
<literal>-1.0</literal>, and <structfield>relallvisible</structfield>
must be greater than or equal to <literal>0</literal>.
--
2.46.2
From f3a3df68763892f966547c2705b0cf9198bd55ec Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huin...@gmail.com>
Date: Wed, 16 Oct 2024 18:45:57 -0400
Subject: [PATCH v2 2/3] Turn off autovacuum for pg_set_relation_stats() tests.
Unlikely as it may be, it is theoretically possible that autovacuum
could act upon one of the tables that have had test stats applied and
set the stats to reflect the empty table that it is. To avoid the
possibility of an intermittent (and very confusing) regression test
failure, turn off autovacuum for all tables that will have stats applied
during the tests.
---
src/test/regress/expected/stats_import.out | 9 ++++++---
src/test/regress/sql/stats_import.sql | 9 ++++++---
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index 46e45a5fa3..de37d890c4 100644
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -11,7 +11,7 @@ CREATE TABLE stats_import.test(
comp stats_import.complex_type,
arange int4range,
tags text[]
-);
+) WITH (autovacuum_enabled = false);
-- starting stats
SELECT relpages, reltuples, relallvisible
FROM pg_class
@@ -136,11 +136,14 @@ SELECT
ERROR: cannot modify statistics for relation "testview"
DETAIL: This operation is not supported for views.
-- Partitioned tables with at least 1 child partition will, when analyzed,
--- have a relpages of -1
+-- have a relpages of -1.
+-- Note: can't set storage params on partitioned tables, so just set for
+-- the child table(s).
CREATE TABLE stats_import.part_parent ( i integer ) PARTITION BY RANGE(i);
CREATE TABLE stats_import.part_child_1
PARTITION OF stats_import.part_parent
- FOR VALUES FROM (0) TO (10);
+ FOR VALUES FROM (0) TO (10)
+ WITH (autovacuum_enabled = false);
ANALYZE stats_import.part_parent;
SELECT relpages
FROM pg_class
diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index ad129b1ab9..b8d1ef795f 100644
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -13,7 +13,7 @@ CREATE TABLE stats_import.test(
comp stats_import.complex_type,
arange int4range,
tags text[]
-);
+) WITH (autovacuum_enabled = false);
-- starting stats
SELECT relpages, reltuples, relallvisible
@@ -96,11 +96,14 @@ SELECT
'stats_import.testview'::regclass);
-- Partitioned tables with at least 1 child partition will, when analyzed,
--- have a relpages of -1
+-- have a relpages of -1.
+-- Note: can't set storage params on partitioned tables, so just set for
+-- the child table(s).
CREATE TABLE stats_import.part_parent ( i integer ) PARTITION BY RANGE(i);
CREATE TABLE stats_import.part_child_1
PARTITION OF stats_import.part_parent
- FOR VALUES FROM (0) TO (10);
+ FOR VALUES FROM (0) TO (10)
+ WITH (autovacuum_enabled = false);
ANALYZE stats_import.part_parent;
--
2.46.2
From cd66d8c4d3874cffe27f63ef7eaedbbce70142af Mon Sep 17 00:00:00 2001
From: Corey Huinker <corey.huin...@gmail.com>
Date: Wed, 16 Oct 2024 19:13:01 -0400
Subject: [PATCH v2 3/3] Allow pg_set_relation_stats to continue after
non-ERROR validation.
Some validation checks would, when elevel was set to WARNING or lower,
immediately return false after the failed check. That isn't the correct
behavior, and we should instead let the function continue in case there
were other valid values that could be set.
---
src/backend/statistics/relation_stats.c | 31 +++++++++----------------
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index b90f70b190..ffc8d4cebd 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -67,7 +67,6 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
bool nulls[3] = {0};
int ncols = 0;
TupleDesc tupdesc;
- HeapTuple newtup;
stats_check_required_arg(fcinfo, relarginfo, RELATION_ARG);
@@ -109,10 +108,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("relpages cannot be < -1")));
table_close(crel, RowExclusiveLock);
- return false;
}
-
- if (relpages != pgcform->relpages)
+ else if (relpages != pgcform->relpages)
{
replaces[ncols] = Anum_pg_class_relpages;
values[ncols] = Int32GetDatum(relpages);
@@ -130,10 +127,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("reltuples cannot be < -1.0")));
table_close(crel, RowExclusiveLock);
- return false;
}
-
- if (reltuples != pgcform->reltuples)
+ else if (reltuples != pgcform->reltuples)
{
replaces[ncols] = Anum_pg_class_reltuples;
values[ncols] = Float4GetDatum(reltuples);
@@ -151,10 +146,8 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("relallvisible cannot be < 0")));
table_close(crel, RowExclusiveLock);
- return false;
}
-
- if (relallvisible != pgcform->relallvisible)
+ else if (relallvisible != pgcform->relallvisible)
{
replaces[ncols] = Anum_pg_class_relallvisible;
values[ncols] = Int32GetDatum(relallvisible);
@@ -163,22 +156,20 @@ relation_statistics_update(FunctionCallInfo fcinfo, int elevel)
}
/* only update pg_class if there is a meaningful change */
- if (ncols == 0)
+ if (ncols > 0)
{
- table_close(crel, RowExclusiveLock);
- return false;
+ HeapTuple newtup;
+
+ newtup = heap_modify_tuple_by_cols(ctup, tupdesc, ncols, replaces, values,
+ nulls);
+ CatalogTupleUpdate(crel, &newtup->t_self, newtup);
+ heap_freetuple(newtup);
}
- newtup = heap_modify_tuple_by_cols(ctup, tupdesc, ncols, replaces, values,
- nulls);
-
- CatalogTupleUpdate(crel, &newtup->t_self, newtup);
- heap_freetuple(newtup);
-
/* release the lock, consistent with vac_update_relstats() */
table_close(crel, RowExclusiveLock);
- return true;
+ return (ncols > 0);
}
/*
--
2.46.2
