From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <8CD3DF77-C4F0-4CA0-B329-2B62B5A85E3B@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_1E3AB177-D6D0-4CC6-9636-E63173A300C5"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Fix pgstat_database.c to honor passed database OIDs
Date: Fri, 10 Apr 2026 16:44:35 +0800
In-Reply-To: <aditMVNDceHtvYj4@paquier.xyz>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
To: Michael Paquier <michael@paquier.xyz>
References: <ABBD5026-506F-4006-A569-28F72C188693@gmail.com>
 <adiU2YZ0auyO6jie@paquier.xyz> <aditMVNDceHtvYj4@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/8CD3DF77-C4F0-4CA0-B329-2B62B5A85E3B%40gmail.com>
Precedence: bulk


--Apple-Mail=_1E3AB177-D6D0-4CC6-9636-E63173A300C5
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On Apr 10, 2026, at 15:56, Michael Paquier <michael@paquier.xyz> =
wrote:
>=20
> 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.
>>=20
>> 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.
>>=20
>> I cannot look at that today, will do so later..
>=20
> -    dbref =3D pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE, =
MyDatabaseId, InvalidOid,
> +    if (!OidIsValid(dboid))
> +        return;
> +
> +    dbref =3D pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE, =
dboid, InvalidOid,
>                                         false);
>=20
> 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);
>=20
> 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.
>=20
> [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/


--Apple-Mail=_1E3AB177-D6D0-4CC6-9636-E63173A300C5
Content-Disposition: attachment;
	filename=v2-0001-Fix-pgstat_database.c-to-honor-passed-database-OI.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v2-0001-Fix-pgstat_database.c-to-honor-passed-database-OI.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=2039e558a01d6cea521fb76172fbda7fefb3bd5f09=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20<lic@highgo.com>=0A=
Date:=20Fri,=2010=20Apr=202026=2011:21:20=20+0800=0ASubject:=20[PATCH=20=
v2]=20Fix=20pgstat_database.c=20to=20honor=20passed=20database=20OIDs=0A=0A=
Several=20functions=20in=20pgstat_database.c=20take=20a=20database=20OID=20=
argument=0Abut=20then=20ignore=20it=20and=20use=20MyDatabaseId=20=
instead.=20That=20is=20confusing,=0Aand=20in=20=
pgstat_reset_database_timestamp()=20it=20is=20plainly=20wrong,=20because=0A=
the=20function's=20contract=20is=20to=20reset=20the=20timestamp=20for=20=
the=20specified=0Adatabase.=0A=0AUse=20the=20passed=20dboid=20in=20=
pgstat_report_connect(),=0Apgstat_report_disconnect(),=20and=20=
pgstat_reset_database_timestamp().=0A=0AAuthor:=20Chao=20Li=20=
<lic@highgo.com>=0AReviewed-by:=20Michael=20Paquier=20=
<michael@paquier.xyz>=0ADiscussion:=20=
https://postgr.es/m/ABBD5026-506F-4006-A569-28F72C188693@gmail.com=0A---=0A=
=20src/backend/utils/activity/pgstat_database.c=20|=20=206=20+++---=0A=20=
src/test/regress/expected/stats.out=20=20=20=20=20=20=20=20=20=20|=2018=20=
++++++++++++++++++=0A=20src/test/regress/sql/stats.sql=20=20=20=20=20=20=20=
=20=20=20=20=20=20=20=20|=20=208=20++++++++=0A=203=20files=20changed,=20=
29=20insertions(+),=203=20deletions(-)=0A=0Adiff=20--git=20=
a/src/backend/utils/activity/pgstat_database.c=20=
b/src/backend/utils/activity/pgstat_database.c=0Aindex=20=
f1846d3236c..7f3bc016593=20100644=0A---=20=
a/src/backend/utils/activity/pgstat_database.c=0A+++=20=
b/src/backend/utils/activity/pgstat_database.c=0A@@=20-243,7=20+243,7=20=
@@=20pgstat_report_connect(Oid=20dboid)=0A=20=0A=20=09=
pgLastSessionReportTime=20=3D=20MyStartTimestamp;=0A=20=0A-=09dbentry=20=
=3D=20pgstat_prep_database_pending(MyDatabaseId);=0A+=09dbentry=20=3D=20=
pgstat_prep_database_pending(dboid);=0A=20=09dbentry->sessions++;=0A=20}=0A=
=20=0A@@=20-258,7=20+258,7=20@@=20pgstat_report_disconnect(Oid=20dboid)=0A=
=20=09if=20(!pgstat_should_report_connstat())=0A=20=09=09return;=0A=20=0A=
-=09dbentry=20=3D=20pgstat_prep_database_pending(MyDatabaseId);=0A+=09=
dbentry=20=3D=20pgstat_prep_database_pending(dboid);=0A=20=0A=20=09=
switch=20(pgStatSessionEndCause)=0A=20=09{=0A@@=20-419,7=20+419,7=20@@=20=
pgstat_reset_database_timestamp(Oid=20dboid,=20TimestampTz=20ts)=0A=20=09=
PgStat_EntryRef=20*dbref;=0A=20=09PgStatShared_Database=20*dbentry;=0A=20=
=0A-=09dbref=20=3D=20pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE,=20=
MyDatabaseId,=20InvalidOid,=0A+=09dbref=20=3D=20=
pgstat_get_entry_ref_locked(PGSTAT_KIND_DATABASE,=20dboid,=20InvalidOid,=0A=
=20=09=09=09=09=09=09=09=09=09=09false);=0A=20=0A=20=09dbentry=20=3D=20=
(PgStatShared_Database=20*)=20dbref->shared_stats;=0Adiff=20--git=20=
a/src/test/regress/expected/stats.out=20=
b/src/test/regress/expected/stats.out=0Aindex=2055009cfcc7d..be2ee02bfa0=20=
100644=0A---=20a/src/test/regress/expected/stats.out=0A+++=20=
b/src/test/regress/expected/stats.out=0A@@=20-924,6=20+924,10=20@@=20=
SELECT=20(n_tup_ins=20+=20n_tup_upd)=20>=200=20AS=20has_data=20FROM=20=
pg_stat_all_tables=0A=20=20t=0A=20(1=20row)=0A=20=0A+SELECT=20=
coalesce(stats_reset::text,=20'NULL')=20AS=20shared_db_reset_before=0A+=20=
=20FROM=20pg_stat_database=20WHERE=20datid=20=3D=200=20\gset=0A+SELECT=20=
coalesce(stats_reset::text,=20'NULL')=20AS=20current_db_reset_before=0A+=20=
=20FROM=20pg_stat_database=20WHERE=20datname=20=3D=20current_database()=20=
\gset=0A=20SELECT=20=
pg_stat_reset_single_table_counters('pg_shdescription'::regclass);=0A=20=20=
pg_stat_reset_single_table_counters=20=0A=20=
-------------------------------------=0A@@=20-937,6=20+941,20=20@@=20=
SELECT=20(n_tup_ins=20+=20n_tup_upd)=20>=200=20AS=20has_data=20FROM=20=
pg_stat_all_tables=0A=20=20f=0A=20(1=20row)=0A=20=0A+SELECT=20=
coalesce(stats_reset::text,=20'NULL')=20<>=20:'shared_db_reset_before'=20=
AS=20shared_db_reset_changed=0A+=20=20FROM=20pg_stat_database=20WHERE=20=
datid=20=3D=200;=0A+=20shared_db_reset_changed=20=0A=
+-------------------------=0A+=20t=0A+(1=20row)=0A+=0A+SELECT=20=
coalesce(stats_reset::text,=20'NULL')=20=3D=20:'current_db_reset_before'=20=
AS=20current_db_reset_not_changed=0A+=20=20FROM=20pg_stat_database=20=
WHERE=20datname=20=3D=20current_database();=0A+=20=
current_db_reset_not_changed=20=0A+------------------------------=0A+=20=
t=0A+(1=20row)=0A+=0A=20--=20set=20back=20comment=0A=20\if=20=
:{?description_before}=0A=20=20=20COMMENT=20ON=20DATABASE=20:"datname"=20=
IS=20:'description_before';=0Adiff=20--git=20=
a/src/test/regress/sql/stats.sql=20b/src/test/regress/sql/stats.sql=0A=
index=20158f3ca6ebe..d4d371f6efa=20100644=0A---=20=
a/src/test/regress/sql/stats.sql=0A+++=20=
b/src/test/regress/sql/stats.sql=0A@@=20-416,9=20+416,17=20@@=20COMMIT;=0A=
=20--=20check=20that=20the=20stats=20are=20reset.=0A=20SELECT=20=
(n_tup_ins=20+=20n_tup_upd)=20>=200=20AS=20has_data=20FROM=20=
pg_stat_all_tables=0A=20=20=20WHERE=20relid=20=3D=20=
'pg_shdescription'::regclass;=0A+SELECT=20coalesce(stats_reset::text,=20=
'NULL')=20AS=20shared_db_reset_before=0A+=20=20FROM=20pg_stat_database=20=
WHERE=20datid=20=3D=200=20\gset=0A+SELECT=20coalesce(stats_reset::text,=20=
'NULL')=20AS=20current_db_reset_before=0A+=20=20FROM=20pg_stat_database=20=
WHERE=20datname=20=3D=20current_database()=20\gset=0A=20SELECT=20=
pg_stat_reset_single_table_counters('pg_shdescription'::regclass);=0A=20=
SELECT=20(n_tup_ins=20+=20n_tup_upd)=20>=200=20AS=20has_data=20FROM=20=
pg_stat_all_tables=0A=20=20=20WHERE=20relid=20=3D=20=
'pg_shdescription'::regclass;=0A+SELECT=20coalesce(stats_reset::text,=20=
'NULL')=20<>=20:'shared_db_reset_before'=20AS=20shared_db_reset_changed=0A=
+=20=20FROM=20pg_stat_database=20WHERE=20datid=20=3D=200;=0A+SELECT=20=
coalesce(stats_reset::text,=20'NULL')=20=3D=20:'current_db_reset_before'=20=
AS=20current_db_reset_not_changed=0A+=20=20FROM=20pg_stat_database=20=
WHERE=20datname=20=3D=20current_database();=0A=20=0A=20--=20set=20back=20=
comment=0A=20\if=20:{?description_before}=0A--=20=0A2.50.1=20(Apple=20=
Git-155)=0A=0A=

--Apple-Mail=_1E3AB177-D6D0-4CC6-9636-E63173A300C5--