Re: Fix pg_stat_reset_single_table_counters function
On 2023-08-21 13:34, Michael Paquier wrote: On Mon, Aug 21, 2023 at 11:33:28AM +0900, Kyotaro Horiguchi wrote: I'm not sure about others, but I would avoid using the name "current_database" for the variable. Agreed. Looking at the surroundings, we have for example "datname" in the collation tests so I have just used that, and applied the patch down to 15. Thanks! Regards, -- Masahiro Ikeda NTT DATA CORPORATION
Re: Fix pg_stat_reset_single_table_counters function
On Mon, Aug 21, 2023 at 11:33:28AM +0900, Kyotaro Horiguchi wrote: > I'm not sure about others, but I would avoid using the name > "current_database" for the variable. Agreed. Looking at the surroundings, we have for example "datname" in the collation tests so I have just used that, and applied the patch down to 15. -- Michael signature.asc Description: PGP signature
Re: Fix pg_stat_reset_single_table_counters function
On 2023-08-21 11:33, Kyotaro Horiguchi wrote:
On 2023/08/15 14:13, Masahiro Ikeda wrote:
On 2023-08-15 11:48, Masahiko Sawada wrote:
+COMMENT ON DATABASE :current_database IS 'This is a test comment';
-- insert or update in 'pg_shdescription'
I think the current_database should be quoted (see other examples
where using current_database(), e.g. collate.linux.utf8.sql). Also it
would be better to reset the comment after the test.
I'm not sure about others, but I would avoid using the name
"current_database" for the variable.
I would just use "database" or "db" instead.
Thanks! I agree your comment.
I fixed in v5 patch.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATIONFrom 5980c041e33fd180e43868cbdc878ac0cf93112e Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda
Date: Mon, 21 Aug 2023 13:24:27 +0900
Subject: [PATCH] Fix pg_stat_reset_single_table_counters function.
This commit revives the feature to reset statistics for a single
relation shared across all databases in the cluster to zero, which
was implemented by the following commit.
* Enhance pg_stat_reset_single_table_counters function(e04267844)
The following commit accidentally deleted the feature.
* pgstat: store statistics in shared memory(5891c7a8e)
Need to backpatch from 15.
Reported-by: Mitsuru Hinata
---
src/backend/utils/adt/pgstatfuncs.c | 9 +--
src/test/regress/expected/stats.out | 42 +
src/test/regress/sql/stats.sql | 26 ++
3 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..2b9742ad21 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlogprefetcher.h"
+#include "catalog/catalog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1776,13 +1777,17 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-/* Reset a single counter in the current database */
+/*
+ * Reset a statistics for a single object, which may be of current
+ * database or shared across all databases in the cluster.
+ */
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
Oid taboid = PG_GETARG_OID(0);
+ Oid dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);
- pgstat_reset(PGSTAT_KIND_RELATION, MyDatabaseId, taboid);
+ pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);
PG_RETURN_VOID();
}
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 319164a5e9..9985579e18 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -764,6 +764,48 @@ FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
2 | t |3 | t
(1 row)
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+-- store the old comment to reset
+SELECT shobj_description(d.oid, 'pg_database') as description_before
+FROM pg_database d WHERE datname = current_database() \gset
+-- update the stats in pg_shdescription
+BEGIN;
+SELECT current_database() as db \gset
+COMMENT ON DATABASE :"db" IS 'This is a test comment';
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush
+--
+
+(1 row)
+
+COMMIT;
+-- check to reset the stats
+SELECT n_tup_ins + n_tup_upd > 0 FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+ t
+(1 row)
+
+SELECT pg_stat_reset_single_table_counters('pg_shdescription'::regclass);
+ pg_stat_reset_single_table_counters
+-
+
+(1 row)
+
+SELECT n_tup_ins + n_tup_upd FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+0
+(1 row)
+
+-- cleanup the comment
+\if :{?description_before}
+ COMMENT ON DATABASE :"db" IS :'description_before';
+\else
+ COMMENT ON DATABASE :"db" IS NULL;
+\endif
-
-- Test that various stats views are being properly populated
-
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 9a16df1c49..5393b18faa 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -376,6 +376,32 @@ COMMIT;
SELECT seq_scan, :'test_last_seq' = last_seq_scan AS seq_ok, idx_scan, :'test_last_idx' < last_idx_scan AS idx_ok
FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+
+-- store the old comment to reset
+SELECT shobj_description(d.oid, 'pg_database') as description_before
+FROM pg_database d WHERE datname = current_database() \gset
+
+-- update the stats in pg_shdescription
Re: Fix pg_stat_reset_single_table_counters function
Apologies. In the previous mail, I mistakenly addressed it to the wrong recipients. Reposted. On 2023/08/15 14:13, Masahiro Ikeda wrote: On 2023-08-15 11:48, Masahiko Sawada wrote: +COMMENT ON DATABASE :current_database IS 'This is a test comment'; -- insert or update in 'pg_shdescription' I think the current_database should be quoted (see other examples where using current_database(), e.g. collate.linux.utf8.sql). Also it would be better to reset the comment after the test. Thanks! I fixed the issues in the v4 patch. I'm not sure about others, but I would avoid using the name "current_database" for the variable. I would just use "database" or "db" instead. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: Fix pg_stat_reset_single_table_counters function
On 2023-08-15 11:48, Masahiko Sawada wrote:
On Mon, Aug 14, 2023 at 5:12 PM Masahiro Ikeda
wrote:
I changed the table to check the stats from pg_database to
pg_shdescription
because the stats can update via the SQL interface COMMENT command.
It seems to work well.
+COMMENT ON DATABASE :current_database IS 'This is a test comment';
-- insert or update in 'pg_shdescription'
I think the current_database should be quoted (see other examples
where using current_database(), e.g. collate.linux.utf8.sql). Also it
would be better to reset the comment after the test.
Thanks! I fixed the issues in the v4 patch.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATIONFrom bca4c1844994be8ed80a29b8cb760e2eb865dca9 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda
Date: Mon, 14 Aug 2023 16:48:30 +0900
Subject: [PATCH] Fix pg_stat_reset_single_table_counters function.
This commit revives the feature to reset statistics for a single
relation shared across all databases in the cluster to zero, which
was implemented by the following commit.
* Enhance pg_stat_reset_single_table_counters function(e04267844)
The following commit accidentally deleted the feature.
* pgstat: store statistics in shared memory(5891c7a8e)
Need to backpatch from 15.
Reported-by: Mitsuru Hinata
---
src/backend/utils/adt/pgstatfuncs.c | 9 +--
src/test/regress/expected/stats.out | 42 +
src/test/regress/sql/stats.sql | 26 ++
3 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..2b9742ad21 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlogprefetcher.h"
+#include "catalog/catalog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1776,13 +1777,17 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-/* Reset a single counter in the current database */
+/*
+ * Reset a statistics for a single object, which may be of current
+ * database or shared across all databases in the cluster.
+ */
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
Oid taboid = PG_GETARG_OID(0);
+ Oid dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);
- pgstat_reset(PGSTAT_KIND_RELATION, MyDatabaseId, taboid);
+ pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);
PG_RETURN_VOID();
}
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 319164a5e9..eb24a02147 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -764,6 +764,48 @@ FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
2 | t |3 | t
(1 row)
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+-- store the old comment to reset
+SELECT shobj_description(d.oid, 'pg_database') as description_before
+FROM pg_database d WHERE datname = current_database() \gset
+-- update the stats in pg_shdescription
+BEGIN;
+SELECT current_database() as current_database \gset
+COMMENT ON DATABASE :"current_database" IS 'This is a test comment';
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush
+--
+
+(1 row)
+
+COMMIT;
+-- check to reset the stats
+SELECT n_tup_ins + n_tup_upd > 0 FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+ t
+(1 row)
+
+SELECT pg_stat_reset_single_table_counters('pg_shdescription'::regclass);
+ pg_stat_reset_single_table_counters
+-
+
+(1 row)
+
+SELECT n_tup_ins + n_tup_upd FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+0
+(1 row)
+
+-- cleanup the comment
+\if :{?description_before}
+ COMMENT ON DATABASE :"current_database" IS :'description_before';
+\else
+ COMMENT ON DATABASE :"current_database" IS NULL;
+\endif
-
-- Test that various stats views are being properly populated
-
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 9a16df1c49..735118c452 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -376,6 +376,32 @@ COMMIT;
SELECT seq_scan, :'test_last_seq' = last_seq_scan AS seq_ok, idx_scan, :'test_last_idx' < last_idx_scan AS idx_ok
FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+
+-- store the old comment to reset
+SELECT shobj_description(d.oid, 'pg_database') as description_before
+FROM pg_database d WHERE datname = current_database() \gset
+
+-- update the stats in pg_shdescri
Re: Fix pg_stat_reset_single_table_counters function
Hi, On Mon, Aug 14, 2023 at 5:12 PM Masahiro Ikeda wrote: > > Hi, > > On 2023-08-13 04:12, Andres Freund wrote: > > On 2023-08-10 17:48:10 +0900, Masahiko Sawada wrote: > >> Good catch! I've confirmed that the issue has been fixed by your > >> patch. > > > > Indeed. > > Thanks for your responses! > > >> However, I'm not sure the added regression tests are stable since > >> autovacuum workers may scan the pg_database and increment the > >> statistics after resetting the stats. > > > > What about updating the table and checking the update count is reset? > > That'd > > not be reset by autovacuum. > > Yes. I confirmed that the stats are incremented by autovacuum as you > said. > > I updated the patch to v3. > * remove the code to bump the CATALOG_VERSION_NO because I misunderstood > * change the test logic to check the update count instead of scan count Thank you for updating the patch! > > I changed the table to check the stats from pg_database to > pg_shdescription > because the stats can update via the SQL interface COMMENT command. It seems to work well. +COMMENT ON DATABASE :current_database IS 'This is a test comment'; -- insert or update in 'pg_shdescription' I think the current_database should be quoted (see other examples where using current_database(), e.g. collate.linux.utf8.sql). Also it would be better to reset the comment after the test. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Fix pg_stat_reset_single_table_counters function
Hi,
On 2023-08-13 04:12, Andres Freund wrote:
On 2023-08-10 17:48:10 +0900, Masahiko Sawada wrote:
Good catch! I've confirmed that the issue has been fixed by your
patch.
Indeed.
Thanks for your responses!
However, I'm not sure the added regression tests are stable since
autovacuum workers may scan the pg_database and increment the
statistics after resetting the stats.
What about updating the table and checking the update count is reset?
That'd
not be reset by autovacuum.
Yes. I confirmed that the stats are incremented by autovacuum as you
said.
I updated the patch to v3.
* remove the code to bump the CATALOG_VERSION_NO because I misunderstood
* change the test logic to check the update count instead of scan count
I changed the table to check the stats from pg_database to
pg_shdescription
because the stats can update via the SQL interface COMMENT command.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATIONFrom 215ef8ef68af30753cfcd4336b1f6bd9203ac014 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda
Date: Mon, 14 Aug 2023 16:48:30 +0900
Subject: [PATCH] Fix pg_stat_reset_single_table_counters function.
This commit revives the feature to reset statistics for a single
relation shared across all databases in the cluster to zero, which
was implemented by the following commit.
* Enhance pg_stat_reset_single_table_counters function(e04267844)
The following commit accidentally deleted the feature.
* pgstat: store statistics in shared memory(5891c7a8e)
Need to backpatch from 15.
Reported-by: Mitsuru Hinata
---
src/backend/utils/adt/pgstatfuncs.c | 9 +++--
src/test/regress/expected/stats.out | 31 +
src/test/regress/sql/stats.sql | 13
3 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..2b9742ad21 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlogprefetcher.h"
+#include "catalog/catalog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1776,13 +1777,17 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-/* Reset a single counter in the current database */
+/*
+ * Reset a statistics for a single object, which may be of current
+ * database or shared across all databases in the cluster.
+ */
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
Oid taboid = PG_GETARG_OID(0);
+ Oid dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);
- pgstat_reset(PGSTAT_KIND_RELATION, MyDatabaseId, taboid);
+ pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);
PG_RETURN_VOID();
}
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 319164a5e9..11cb841386 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -764,6 +764,37 @@ FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
2 | t |3 | t
(1 row)
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+BEGIN;
+SELECT current_database() as current_database \gset
+COMMENT ON DATABASE :current_database IS 'This is a test comment'; -- insert or update in 'pg_shdescription'
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush
+--
+
+(1 row)
+
+COMMIT;
+SELECT n_tup_ins + n_tup_upd > 0 FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+ t
+(1 row)
+
+SELECT pg_stat_reset_single_table_counters('pg_shdescription'::regclass);
+ pg_stat_reset_single_table_counters
+-
+
+(1 row)
+
+SELECT n_tup_ins + n_tup_upd FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+ ?column?
+--
+0
+(1 row)
+
-
-- Test that various stats views are being properly populated
-
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 9a16df1c49..d113aed257 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -376,6 +376,19 @@ COMMIT;
SELECT seq_scan, :'test_last_seq' = last_seq_scan AS seq_ok, idx_scan, :'test_last_idx' < last_idx_scan AS idx_ok
FROM pg_stat_all_tables WHERE relid = 'test_last_scan'::regclass;
+-
+-- Test to reset stats for a table shared across all databases (ex. pg_shdescription)
+-
+
+BEGIN;
+SELECT current_database() as current_database \gset
+COMMENT ON DATABASE :current_database IS 'This is a test comment'; -- insert or update in 'pg_shdescription'
+SELECT pg_stat_force_next_flush();
+COMMIT;
+
+SELECT n_tup_ins + n_tup_upd > 0 FROM pg_stat_all_tables WHERE relid = 'pg_shdescription'::regclass;
+SELECT pg_stat_reset_single_t
Re: Fix pg_stat_reset_single_table_counters function
Hi, On 2023-08-10 17:48:10 +0900, Masahiko Sawada wrote: > Good catch! I've confirmed that the issue has been fixed by your patch. Indeed. > However, I'm not sure the added regression tests are stable since > autovacuum workers may scan the pg_database and increment the > statistics after resetting the stats. What about updating the table and checking the update count is reset? That'd not be reset by autovacuum. Greetings, Andres Freund
Re: Fix pg_stat_reset_single_table_counters function
On Thu, Aug 10, 2023 at 2:10 PM Masahiro Ikeda wrote: > > On 2023-08-01 15:23, Masahiro Ikeda wrote: > > Hi, > > > > My colleague, Mitsuru Hinata (in CC), found the following issue. > > > > The documentation of pg_stat_reset_single_table_counters() says > >> pg_stat_reset_single_table_counters ( oid ) → void > >> Resets statistics for a single table or index in the current database > >> or shared across all databases in the cluster to zero. > >> This function is restricted to superusers by default, but other users > >> can be granted EXECUTE to run the function. > > https://www.postgresql.org/docs/devel/monitoring-stats.html > > > > But, the stats will not be reset if the table shared across all > > databases is specified. IIUC, 5891c7a8e seemed to have mistakenly > > removed the feature implemented in e04267844. What do you think? Good catch! I've confirmed that the issue has been fixed by your patch. However, I'm not sure the added regression tests are stable since autovacuum workers may scan the pg_database and increment the statistics after resetting the stats. Also, the patch bumps the CATALOG_VERSION_NO but I don't think it's necessary. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
Re: Fix pg_stat_reset_single_table_counters function
On 2023-08-01 15:23, Masahiro Ikeda wrote:
Hi,
My colleague, Mitsuru Hinata (in CC), found the following issue.
The documentation of pg_stat_reset_single_table_counters() says
pg_stat_reset_single_table_counters ( oid ) → void
Resets statistics for a single table or index in the current database
or shared across all databases in the cluster to zero.
This function is restricted to superusers by default, but other users
can be granted EXECUTE to run the function.
https://www.postgresql.org/docs/devel/monitoring-stats.html
But, the stats will not be reset if the table shared across all
databases is specified. IIUC, 5891c7a8e seemed to have mistakenly
removed the feature implemented in e04267844. What do you think?
I fix an issue with the v1 patch reported by cfbot.
The test case didn't assume the number of databases will change in
the test of pg_upgrade.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATIONFrom 182c676380aa91a356814d8e387e5e5fe7e2c3c0 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda
Date: Thu, 10 Aug 2023 13:46:09 +0900
Subject: [PATCH] Fix pg_stat_reset_single_table_counters function.
This commit revives the feature to reset statistics for a single
relation shared across all databases in the cluster to zero, which
was implemented by the following commit.
* Enhance pg_stat_reset_single_table_counters function(e04267844)
The following commit accidentally deleted the feature.
* pgstat: store statistics in shared memory(5891c7a8e)
Bump catalog version.
Need to backpatch from 15.
Reported-by: Mitsuru Hinata
---
src/backend/utils/adt/pgstatfuncs.c | 9 -
src/include/catalog/catversion.h| 2 +-
src/test/regress/expected/stats.out | 60 +
src/test/regress/sql/stats.sql | 18 +
4 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..2b9742ad21 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlogprefetcher.h"
+#include "catalog/catalog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1776,13 +1777,17 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-/* Reset a single counter in the current database */
+/*
+ * Reset a statistics for a single object, which may be of current
+ * database or shared across all databases in the cluster.
+ */
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
Oid taboid = PG_GETARG_OID(0);
+ Oid dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);
- pgstat_reset(PGSTAT_KIND_RELATION, MyDatabaseId, taboid);
+ pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);
PG_RETURN_VOID();
}
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index f507b49bb2..7d20c58b1d 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* mmddN */
-#define CATALOG_VERSION_NO 202307261
+#define CATALOG_VERSION_NO 202308101
#endif
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 319164a5e9..248e072395 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -592,6 +592,66 @@ SELECT seq_scan, idx_scan FROM pg_stat_all_tables WHERE relid = 'test_last_scan'
0 |0
(1 row)
+-- ensure to reset statistics for a table and a index shared across all databases
+BEGIN;
+SET LOCAL enable_seqscan TO on;
+SET LOCAL enable_indexscan TO off;
+SET LOCAL enable_indexonlyscan TO off;
+EXPLAIN (COSTS off) SELECT count(*) >= 0 FROM pg_database;
+ QUERY PLAN
+---
+ Aggregate
+ -> Seq Scan on pg_database
+(2 rows)
+
+SELECT count(*) >= 0 FROM pg_database; -- increment stats for the table
+ ?column?
+--
+ t
+(1 row)
+
+SET LOCAL enable_seqscan TO off;
+SET LOCAL enable_indexscan TO on;
+SET LOCAL enable_indexonlyscan TO off;
+EXPLAIN (COSTS off) SELECT count(*) >= 0 FROM pg_database WHERE oid = 1;
+ QUERY PLAN
+-
+ Aggregate
+ -> Index Scan using pg_database_oid_index on pg_database
+ Index Cond: (oid = '1'::oid)
+(3 rows)
+
+SELECT count(*) >= 0 FROM pg_database WHERE oid = 1; -- increment stats for the index
+ ?column?
+--
+ t
+(1 row)
+
+COMMIT;
+SELECT pg_stat_reset_single_table_counters('pg_database'::regclass);
+ pg_stat_reset_single_table_counters
+-
+
+(1 row)
+
+SELECT pg_stat_reset_single_table_counters('pg_database_oid_index'::regclass);
+ pg_stat_reset_single_table_counters
+--
Fix pg_stat_reset_single_table_counters function
Hi,
My colleague, Mitsuru Hinata (in CC), found the following issue.
The documentation of pg_stat_reset_single_table_counters() says
pg_stat_reset_single_table_counters ( oid ) → void
Resets statistics for a single table or index in the current database
or shared across all databases in the cluster to zero.
This function is restricted to superusers by default, but other users
can be granted EXECUTE to run the function.
https://www.postgresql.org/docs/devel/monitoring-stats.html
But, the stats will not be reset if the table shared across all
databases is specified. IIUC, 5891c7a8e seemed to have mistakenly
removed the feature implemented in e04267844. What do you think?
* reproduce procedure
SELECT COUNT(*) FROM pg_stat_database;
SELECT pg_stat_reset_single_table_counters('pg_database'::regclass);
SELECT seq_scan FROM pg_stat_all_tables WHERE relid =
'pg_database'::regclass;
* unexpected result
* Rename OverrideSearchPath to SearchPathMatcher (current HEAD:
d3a38318a)
* pgstat: store statistics in shared memory (5891c7a8e)
psql=# SELECT seq_scan FROM pg_stat_all_tables WHERE relid =
'pg_database'::regclass;
seq_scan
--
11
(1 row)
* expected result
* Enhance pg_stat_reset_single_table_counters function (e04267844)
* pgstat: normalize function naming (be902e2651), which is previous
commit of 5891c7a8e.
psql=# SELECT seq_scan FROM pg_stat_all_tables WHERE relid =
'pg_database'::regclass;
seq_scan
--
0
(1 row)
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATIONFrom 561c4391c9dac30b5478637a6baf8c8689226da5 Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda
Date: Tue, 1 Aug 2023 13:46:00 +0900
Subject: [PATCH] Fix pg_stat_reset_single_table_counters function.
This commit revives the feature to reset statistics for a single
relation shared across all databases in the cluster to zero, which
was implemented by the following commit.
* Enhance pg_stat_reset_single_table_counters function(e04267844)
The following commit accidentally deleted the feature.
* pgstat: store statistics in shared memory(5891c7a8e)
Bump catalog version.
Need to backpatch from 15.
Reported-by: Mitsuru Hinata
---
src/backend/utils/adt/pgstatfuncs.c | 9 -
src/include/catalog/catversion.h| 2 +-
src/test/regress/expected/stats.out | 60 +
src/test/regress/sql/stats.sql | 18 +
4 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..2b9742ad21 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -17,6 +17,7 @@
#include "access/htup_details.h"
#include "access/xlog.h"
#include "access/xlogprefetcher.h"
+#include "catalog/catalog.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "common/ip.h"
@@ -1776,13 +1777,17 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
-/* Reset a single counter in the current database */
+/*
+ * Reset a statistics for a single object, which may be of current
+ * database or shared across all databases in the cluster.
+ */
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
Oid taboid = PG_GETARG_OID(0);
+ Oid dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);
- pgstat_reset(PGSTAT_KIND_RELATION, MyDatabaseId, taboid);
+ pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);
PG_RETURN_VOID();
}
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index f507b49bb2..5a534771ed 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* mmddN */
-#define CATALOG_VERSION_NO 202307261
+#define CATALOG_VERSION_NO 202308011
#endif
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 8e63340782..23450d28a8 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -592,6 +592,66 @@ SELECT seq_scan, idx_scan FROM pg_stat_all_tables WHERE relid = 'test_last_scan'
0 |0
(1 row)
+-- ensure to reset statistics for a table and a index shared across all databases
+BEGIN;
+SET LOCAL enable_seqscan TO on;
+SET LOCAL enable_indexscan TO off;
+SET LOCAL enable_indexonlyscan TO off;
+EXPLAIN (COSTS off) SELECT count(*) FROM pg_database;
+ QUERY PLAN
+---
+ Aggregate
+ -> Seq Scan on pg_database
+(2 rows)
+
+SELECT count(*) FROM pg_database; -- increment stats for the table
+ count
+---
+ 4
+(1 row)
+
+SET LOCAL enable_seqscan TO off;
+SET LOCAL enable_indexscan TO on;
+SET LOCAL enable_indexonlyscan TO off;
+EXPLAIN (COSTS off) SELECT count(*) FROM pg_database WHERE oid = 1;
+ QUERY PLAN
+-
