| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix pgstat_database.c to honor passed database OIDs |
| Date: | 2026-04-10 08:44:35 |
| Message-ID: | 8CD3DF77-C4F0-4CA0-B329-2B62B5A85E3B@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Apr 10, 2026, at 15:56, Michael Paquier <michael(at)paquier(dot)xyz> 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/
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Fix-pgstat_database.c-to-honor-passed-database-OI.patch | application/octet-stream | 4.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-04-10 08:48:26 | var_is_nonnullable() fails to handle invalid NOT NULL constraints |
| Previous Message | Evgeny Voropaev | 2026-04-10 08:44:07 | Re: Compress prune/freeze records with Delta Frame of Reference algorithm |