On Tue, Oct 15, 2024 at 2:50 PM Corey Huinker <corey.huin...@gmail.com>
wrote:
>
> Oh, I see. It appears that there's a special case for partitioned
>> tables that sets relpages=-1 in do_analyze_rel() around line 680. It's
>> a bit inconsistent, though, because even partitioned indexes have
>> relpages=0. Furthermore, the parameter is of type BlockNumber, so
>> InvalidBlockNumber would make more sense.
>>
>> Not the cleanest code, but if the value exists, we need to be able to
>> import it.
>>
>>
> Thanks for tracking that down. I'll have a patch ready shortly.
>
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.
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 v1] 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
