public inbox for [email protected]  
help / color / mirror / Atom feed
From: Dapeng Wang <[email protected]>
To: Chao Li <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Fix pgstat_database.c to honor passed database OIDs
Date: Fri, 10 Apr 2026 21:01:32 +0800
Message-ID: <CAKx0YhzpuMkVwxa-fh36YOk11KZ6tjXnApAF=H6Sgso5a+P4Zg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Chao Li <[email protected]> 于2026年4月10日周五 16:45写道:

>
>
> > On Apr 10, 2026, at 15:56, Michael Paquier <[email protected]> wrote:
> >
> > On Fri, Apr 10, 2026 at 03:12:41PM +0900, Michael Paquier wrote:
> >> If we decide to expand pgstat_reset() in other contexts in the
> >> back-branches, we'd be silently trapped as well.
> >>
> >> The connect and disconnect calls are less critical, perhaps we could
> >> remove the argument altogether, but I cannot get excited about that
> >> either as some extensions may rely on these as currently designed.
> >>
> >> I cannot look at that today, will do so later..
> >
> > -    dbref = pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE,
> MyDatabaseId, InvalidOid,
> > +    if (!OidIsValid(dboid))
> > +        return;
> > +
> > +    dbref = pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE, dboid,
> InvalidOid,
> >                                         false);
> >
> > This bypass of an invalid database OID is actually incorrect in the
> > patch.  There is a stats entry with a database OID of 0, documented as
> > such in [1] for shared objects.  There is one test in the main
> > regression test suite that triggers this case:
> > SELECT pg_stat_reset_single_table_counters('pg_shdescription'::regclass);
> >
> > The short answer is to remove this check based on OidIsValid(), and
> > allow the timestamp be reset for this entry of 0 rather than ignore
> > the update.
> >
> > [1]:
> https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-PG-STAT-DATABASE-VIEW
> > --
> > Michael
>
> Thanks for pointing out the test. I badly excluded *.sql and *.out in my
> vscode search last time.
>
> Then the question becomes why the test didn't fail. I looked into it, and
> it seems the existing test does not check the stats_reset timestamp. I have
> now added checks for the stats_reset of both database 0 and the current
> database.
>
> With those checks in place, the test fails without this patch, and also
> fails with the incorrect OidIsValid(dboid) check in v1. With the v2 patch,
> the test passes.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
>
>
Hi Evan,

I tested the v2 patch on HEAD. It applies cleanly, compiles
without warnings, and all 247 regression tests pass.

I also manually verified the bug fix by comparing behavior
before and after the patch:

Without the patch, calling
pg_stat_reset_single_table_counters('pg_shdescription'::regclass)
incorrectly updates the current database's stats_reset timestamp
while leaving the shared db entry (datid=0) unchanged.

With the patch, the shared db entry's stats_reset is correctly
updated, and the current database's timestamp is not affected.

Regards,
Dapeng Wang


view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix pgstat_database.c to honor passed database OIDs
  In-Reply-To: <CAKx0YhzpuMkVwxa-fh36YOk11KZ6tjXnApAF=H6Sgso5a+P4Zg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox