Обсуждение: Add missing stats_reset column to pg_stat_database_conflicts view
Hi hackers, Currently, pg_stat_database and pg_stat_database_conflicts are decoupled into two separate views. However, there is an inconsistency: pg_stat_database_conflicts is missing the stats_reset column. Implementation wise, both views expose data from PgStat_StatDBEntry and share the same reset lifecycle when pg_stat_reset() is called. For now, users monitoring recovery conflicts have to get the reset time from pg_stat_database. I would like these two views to be purely decoupled to avoid such confusion. The attached patch adds pg_stat_get_db_stat_reset_time() to fix this inconsistency. Please let me know your thoughts. Thanks, Shihao
Вложения
> The attached patch adds pg_stat_get_db_stat_reset_time() to fix this > inconsistency. > > Please let me know your thoughts. This view was introduced 15 years ago, and surprisingly this is the first complaint about this. I am also not very surprised. I also noticed that pg_statio_all_sequences does not have a reset column. We should fix this one also. What do you think? With regards to your attached patch, it's missing the doc changes. but also the patch does not apply. -- Sami Imseih Amazon Web Services (AWS)
On Tue, Mar 10, 2026 at 8:33 AM Sami Imseih <samimseih@gmail.com> wrote: > > > The attached patch adds pg_stat_get_db_stat_reset_time() to fix this > > inconsistency. > > > > Please let me know your thoughts. > > This view was introduced 15 years ago, and surprisingly this is the > first complaint about this. I am also not very surprised. > > I also noticed that pg_statio_all_sequences does not have a reset > column. We should fix this one also. What do you think? +1 Also it might be better to update the docs together so that the description of pg_stat_reset_single_table_counters mentions sequences in addition to tables and indexes. Regards, -- Fujii Masao
On Tue, 10 Mar 2026 at 06:17, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Tue, Mar 10, 2026 at 8:33 AM Sami Imseih <samimseih@gmail.com> wrote: > > > > > The attached patch adds pg_stat_get_db_stat_reset_time() to fix this > > > inconsistency. > > > > > > Please let me know your thoughts. > > > > This view was introduced 15 years ago, and surprisingly this is the > > first complaint about this. I am also not very surprised. > > > > I also noticed that pg_statio_all_sequences does not have a reset > > column. We should fix this one also. What do you think? > > +1 > > Also it might be better to update the docs together so that the description of > pg_stat_reset_single_table_counters mentions sequences in addition to > tables and indexes. > > Regards, > > -- > Fujii Masao > Well, it looks like there are not so many users of this view. Anyway, +1 on change. This also need catversion bump -- Best regards, Kirill Reshke
The patch has been rebased, and the documentation and catversion updates have been added. > I also noticed that pg_statio_all_sequences does not have a reset > column. We should fix this one also. What do you think? Right now the pg_statio_all_tables, pg_statio_all_indexes, pg_statio_all_sequences, pg_stat_user_functions all do not have reset_stat supported. I am actively working on tadd a reset_stat support for these view. For now, let's quickly address the db conflict first. On Tue, Mar 10, 2026 at 7:32 AM Kirill Reshke <reshkekirill@gmail.com> wrote: > > On Tue, 10 Mar 2026 at 06:17, Fujii Masao <masao.fujii@gmail.com> wrote: > > > > On Tue, Mar 10, 2026 at 8:33 AM Sami Imseih <samimseih@gmail.com> wrote: > > > > > > > The attached patch adds pg_stat_get_db_stat_reset_time() to fix this > > > > inconsistency. > > > > > > > > Please let me know your thoughts. > > > > > > This view was introduced 15 years ago, and surprisingly this is the > > > first complaint about this. I am also not very surprised. > > > > > > I also noticed that pg_statio_all_sequences does not have a reset > > > column. We should fix this one also. What do you think? > > > > +1 > > > > Also it might be better to update the docs together so that the description of > > pg_stat_reset_single_table_counters mentions sequences in addition to > > tables and indexes. > > > > Regards, > > > > -- > > Fujii Masao > > > > Well, it looks like there are not so many users of this view. Anyway, > +1 on change. > This also need catversion bump > > -- > Best regards, > Kirill Reshke
Вложения
Hi, Please don't top-post. it makes following the thread difficult. > > I also noticed that pg_statio_all_sequences does not have a reset > > column. We should fix this one also. What do you think? > > Right now the pg_statio_all_tables, pg_statio_all_indexes, > pg_statio_all_sequences, pg_stat_user_functions all do not have > reset_stat supported. I am actively working on tadd a reset_stat > support for these view. For now, let's quickly address the db conflict > first. It looks like stats_reset for pg_stat_user_functions was added in b71bae41a0cd and for the others you mention, expected for pg_statio_all_sequences, was added in a5b543258aa2. These are already targeted for 19, and you can also see that in the devel docs [1]. The changes you attached in v2 look good to me, but I think we should also add a test in stats.sql as well. FWIW, I find using "git format-patch" better for the threads. It forces you to write a commit message and properly formats the patch name [2]? It's the most common way I see patches being submitted. [1] [https://www.postgresql.org/docs/devel/monitoring-stats.html] [2] [https://wiki.postgresql.org/wiki/Submitting_a_Patch] -- Sami Imseih Amazon Web Services (AWS).
On Tue, Mar 10, 2026 at 3:27 PM Sami Imseih <samimseih@gmail.com> wrote: > > Hi, > > Please don't top-post. it makes following the thread difficult. > > > > I also noticed that pg_statio_all_sequences does not have a reset > > > column. We should fix this one also. What do you think? > > > > Right now the pg_statio_all_tables, pg_statio_all_indexes, > > pg_statio_all_sequences, pg_stat_user_functions all do not have > > reset_stat supported. I am actively working on tadd a reset_stat > > support for these view. For now, let's quickly address the db conflict > > first. > > It looks like stats_reset for pg_stat_user_functions was added in > b71bae41a0cd and for the others you mention, expected for > pg_statio_all_sequences, was added in a5b543258aa2. > These are already targeted for 19, and you can also see > that in the devel docs [1]. > > The changes you attached in v2 look good to me, but I think > we should also add a test in stats.sql as well. > > FWIW, I find using "git format-patch" better for the threads. It > forces you to write a commit message and properly formats > the patch name [2]? It's the most common way I see patches > being submitted. > > [1] [https://www.postgresql.org/docs/devel/monitoring-stats.html] > [2] [https://wiki.postgresql.org/wiki/Submitting_a_Patch] > > -- > Sami Imseih > Amazon Web Services (AWS). Thanks for pointing that out. I've added new tests and used git format-patch to generate a new patch.
Вложения
On Wed, Mar 11, 2026 at 11:10 PM shihao zhong <zhong950419@gmail.com> wrote: > Thanks for pointing that out. I've added new tests and used git > format-patch to generate a new patch. Thanks for updating the patch! +-- Test that the stats_reset column in pg_stat_database_conflicts is correctly maintained +SELECT pg_stat_reset(); +SELECT stats_reset IS NOT NULL AS has_stats_reset + FROM pg_stat_database_conflicts WHERE datname = current_database(); Since stats.sql already includes tests verifying that reset works for pg_stat_database, it might be better to add the test for pg_stat_database_conflicts alongside those, rather than at the end of stats.sql. Thought? The attached updated patch does that. I also fixed some indentation issues in the docs in the patch. Regards, -- Fujii Masao
Вложения
> On Mar 12, 2026, at 12:50, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Wed, Mar 11, 2026 at 11:10 PM shihao zhong <zhong950419@gmail.com> wrote: >> Thanks for pointing that out. I've added new tests and used git >> format-patch to generate a new patch. > > Thanks for updating the patch! > > +-- Test that the stats_reset column in pg_stat_database_conflicts is > correctly maintained > +SELECT pg_stat_reset(); > +SELECT stats_reset IS NOT NULL AS has_stats_reset > + FROM pg_stat_database_conflicts WHERE datname = current_database(); > > Since stats.sql already includes tests verifying that reset works for > pg_stat_database, it might be better to add the test for > pg_stat_database_conflicts alongside those, rather than at the end of > stats.sql. Thought? The attached updated patch does that. > > I also fixed some indentation issues in the docs in the patch. > > Regards, > > -- > Fujii Masao > <v4-0001-Add-stats_reset-column-to-pg_stat_database_confli.patch> V4 overall LGTM. A couple of nitpicks: 1 - stats.sql ``` +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname \gset +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname; ``` (SELECT current_database()) can be just current_database(). I tried to fix this in my local, and the fix works for me. 2 - stats.sql ``` +-- Since stats_reset in pg_stat_database and pg_stat_database_conflicts starts +-- out as NULL, reset it once first so we have something to compare it to ``` I think “starts” should be “start”, because there are two stats_reset fields from two tables in this context. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Thanks for the updated patch! --- Since pg_stat_database stats_reset starts out as NULL, reset it once first so we have something to compare it to +-- Since stats_reset in pg_stat_database and pg_stat_database_conflicts starts +-- out as NULL, reset it once first so we have something to compare it to SELECT pg_stat_reset(); -SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset +SELECT D.stats_reset AS db_reset_ts, DC.stats_reset AS dbc_reset_ts +FROM pg_stat_database D, pg_stat_database_conflicts DC +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname \gset SELECT pg_stat_reset(); -SELECT stats_reset > :'db_reset_ts'::timestamptz FROM pg_stat_database WHERE datname = (SELECT current_database()); +SELECT D.stats_reset > :'db_reset_ts'::timestamptz, + DC.stats_reset > :'dbc_reset_ts'::timestamptz, + D.stats_reset = DC.stats_reset +FROM pg_stat_database D, pg_stat_database_conflicts DC +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname; I think the changes to the test are overcomplicated unnecessarly. We should not have to join pg_stat_database and pg_stat_database_conflicts. We can just query each fo the stats_reset separately. see v5, I kept the check to ensure that pg_stat_database and pg_stat_database_conflicts have the same reset time. I think this is good to have as well. I also updated the existing comment for more clarity. -- Sami Imseih Amazon Web Services (AWS)
Вложения
On Fri, Mar 13, 2026 at 1:21 AM Sami Imseih <samimseih@gmail.com> wrote: > > Thanks for the updated patch! > > --- Since pg_stat_database stats_reset starts out as NULL, reset it > once first so we have something to compare it to > +-- Since stats_reset in pg_stat_database and pg_stat_database_conflicts starts > +-- out as NULL, reset it once first so we have something to compare it to > SELECT pg_stat_reset(); > -SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname > = (SELECT current_database()) \gset > +SELECT D.stats_reset AS db_reset_ts, DC.stats_reset AS dbc_reset_ts > +FROM pg_stat_database D, pg_stat_database_conflicts DC > +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname \gset > SELECT pg_stat_reset(); > -SELECT stats_reset > :'db_reset_ts'::timestamptz FROM > pg_stat_database WHERE datname = (SELECT current_database()); > +SELECT D.stats_reset > :'db_reset_ts'::timestamptz, > + DC.stats_reset > :'dbc_reset_ts'::timestamptz, > + D.stats_reset = DC.stats_reset > +FROM pg_stat_database D, pg_stat_database_conflicts DC > +WHERE D.datname = (SELECT current_database()) AND D.datname = DC.datname; > > I think the changes to the test are overcomplicated unnecessarly. We should not > have to join pg_stat_database and pg_stat_database_conflicts. We can just > query each fo the stats_reset separately. see v5, I kept the check to > ensure that > pg_stat_database and pg_stat_database_conflicts have the same reset > time. I think > this is good to have as well. I also updated the existing comment for > more clarity. Thanks for updating the patch! I like the simplified test. +SELECT stats_reset AS dbc_reset_ts FROM pg_stat_database_conflicts WHERE datname = (SELECT current_database()) \gset I removed the extra space. I also made some cosmetic indentation fixes in the docs. The v6 patch is attached. Unless there are objections, I'll update the catversion and commit it. Regards, -- Fujii Masao
Вложения
> I also made some cosmetic indentation fixes in the docs. > The v6 patch is attached. Just one more minor comment fix I noticed. "Test that reset works for pg_stat_database" to: "Test that reset works for pg_stat_database and pg_stat_database_conflicts" -- Sami
Вложения
> On Mar 13, 2026, at 04:39, Sami Imseih <samimseih@gmail.com> wrote: > >> I also made some cosmetic indentation fixes in the docs. >> The v6 patch is attached. > > Just one more minor comment fix I noticed. > > "Test that reset works for pg_stat_database" > > to: > > "Test that reset works for pg_stat_database and pg_stat_database_conflicts" > > -- > Sami > <v7-0001-Add-stats_reset-column-to-pg_stat_database_confli.patch> I still saw (SELECT current_database()) in v7. Does it have any special meaning than just current_database()? Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Fri, Mar 13, 2026 at 5:47 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > > > On Mar 13, 2026, at 04:39, Sami Imseih <samimseih@gmail.com> wrote: > > > >> I also made some cosmetic indentation fixes in the docs. > >> The v6 patch is attached. > > > > Just one more minor comment fix I noticed. > > > > "Test that reset works for pg_stat_database" > > > > to: > > > > "Test that reset works for pg_stat_database and pg_stat_database_conflicts" > > > > -- > > Sami > > <v7-0001-Add-stats_reset-column-to-pg_stat_database_confli.patch> I've pushed the patch. Thanks! > I still saw (SELECT current_database()) in v7. Does it have any special meaning than just current_database()? No. I think it was used by the patch simply because the existing tests for stats reset already use it. Regards, -- Fujii Masao
> On Mar 13, 2026, at 21:25, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Mar 13, 2026 at 5:47 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>
>>
>>
>>> On Mar 13, 2026, at 04:39, Sami Imseih <samimseih@gmail.com> wrote:
>>>
>>>> I also made some cosmetic indentation fixes in the docs.
>>>> The v6 patch is attached.
>>>
>>> Just one more minor comment fix I noticed.
>>>
>>> "Test that reset works for pg_stat_database"
>>>
>>> to:
>>>
>>> "Test that reset works for pg_stat_database and pg_stat_database_conflicts"
>>>
>>> --
>>> Sami
>>> <v7-0001-Add-stats_reset-column-to-pg_stat_database_confli.patch>
>
> I've pushed the patch. Thanks!
>
>
>> I still saw (SELECT current_database()) in v7. Does it have any special meaning than just current_database()?
>
> No. I think it was used by the patch simply because the existing tests
> for stats reset already use it.
>
> Regards,
>
> --
> Fujii Masao
Hi Fujii-san,
If the only reason for using (SELECT current_database()) is to follow the existing code, would it make sense to update
thecode to use current_database() instead?
Looking at the execution plans below:
```
evantest=# explain select (select current_database());
QUERY PLAN
---------------------------------------------------
Result (cost=0.01..0.02 rows=1 width=64)
InitPlan expr_1
-> Result (cost=0.00..0.01 rows=1 width=64)
(3 rows)
evantest=# explain select current_database();
QUERY PLAN
-------------------------------------------
Result (cost=0.00..0.01 rows=1 width=64)
(1 row)
```
The nested SELECT introduces an unnecessary InitPlan, which adds extra work to execute.
Another point is that the nested SELECT may mislead users or code readers into thinking it is required. For example, I
oftenlook at regression test scripts to check how SQL statements are used in practice, and I consider them an important
sourceof reference. Because of that, I think it's better for the test scripts to demonstrate the simplest and most
appropriateusage when possible.
I searched through the test scripts, and it looks like only stats.sql uses this pattern. So I attached a small patch
thatreplaces all (SELECT current_database()) with current_database().
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/