Обсуждение: Add missing stats_reset column to pg_stat_database_conflicts view

Поиск
Список
Период
Сортировка

Add missing stats_reset column to pg_stat_database_conflicts view

От
shihao zhong
Дата:
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

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Sami Imseih
Дата:
> 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)



Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Fujii Masao
Дата:
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



Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Kirill Reshke
Дата:
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



Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
shihao zhong
Дата:
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

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Sami Imseih
Дата:
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).



Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
shihao zhong
Дата:
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.

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Fujii Masao
Дата:
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

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Chao Li
Дата:

> 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/







Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Sami Imseih
Дата:
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)

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Fujii Masao
Дата:
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

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Sami Imseih
Дата:
> 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

Вложения

Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Chao Li
Дата:

> 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/







Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Fujii Masao
Дата:
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



Re: Add missing stats_reset column to pg_stat_database_conflicts view

От
Chao Li
Дата:

> 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/





Вложения