public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Fujii Masao <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: shihao zhong <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Add missing stats_reset column to pg_stat_database_conflicts view
Date: Mon, 16 Mar 2026 10:10:34 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHGQGwG2OpV-Z-8d9K1owosKhsDKO2kiJVvvkEzWoOMp3NzAcQ@mail.gmail.com>
References: <CAGRkXqS98OebEWjax99_LVAECsxCB8i=BfsdAL34i-5QHfwyOQ@mail.gmail.com>
<CAA5RZ0vSjCEAuKXhxacnp0ftrYkf9QRcyPK-AvTi_otFVDWphA@mail.gmail.com>
<CAHGQGwEeGUQtj7TOznM2_O2uV-g6iOa1UBBTBfyugJ-nSoS89Q@mail.gmail.com>
<CALdSSPi4apueVqgnJEZqpy6O0HSEPUiUzf=vTLPPijsQLob2Eg@mail.gmail.com>
<CAGRkXqQ0qBHTcD=UZ7_GTLsgn+W-=1c8suMJ3yWZb2eP4m0fBg@mail.gmail.com>
<CAA5RZ0ta3y==2AStFyo-SYyJ2ztd3ZiurN1HQR9gzDTKVtgUDg@mail.gmail.com>
<CAGRkXqRVDuhXNCvQ=j-+5KCu+Gcar_h5OeHTaMaVKF-FLV2YvQ@mail.gmail.com>
<CAHGQGwHaHnRrz9hqpDdn984mqToJDdkjMRVvEXXgFwCnXJb89Q@mail.gmail.com>
<[email protected]>
<CAA5RZ0uStx0cWUCNPDPhBprAVmcdOX-uuKd9B_W9qLa6Tsp46w@mail.gmail.com>
<CAHGQGwF3=y-j5LxHdXtthEsmJYY5U6WtcAsgwbestfCtNvvuaw@mail.gmail.com>
<CAA5RZ0uy79o0vr2=LeRHS2MZ7V6HGOCsys6YAbmmj3BW8tnziw@mail.gmail.com>
<[email protected]>
<CAHGQGwG2OpV-Z-8d9K1owosKhsDKO2kiJVvvkEzWoOMp3NzAcQ@mail.gmail.com>
> On Mar 13, 2026, at 21:25, Fujii Masao <[email protected]> wrote:
>
> On Fri, Mar 13, 2026 at 5:47 PM Chao Li <[email protected]> wrote:
>>
>>
>>
>>> On Mar 13, 2026, at 04:39, Sami Imseih <[email protected]> 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 the code 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 often look at regression test scripts to check how SQL statements are used in practice, and I consider them an important source of reference. Because of that, I think it's better for the test scripts to demonstrate the simplest and most appropriate usage when possible.
I searched through the test scripts, and it looks like only stats.sql uses this pattern. So I attached a small patch that replaces all (SELECT current_database()) with current_database().
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-regress-remove-unnecessary-subquery-around-curren.patch (4.9K, 2-v1-0001-regress-remove-unnecessary-subquery-around-curren.patch)
download | inline diff:
From 52891896d06c9ffba01dde369d47cf02f441185f Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Mon, 16 Mar 2026 10:01:11 +0800
Subject: [PATCH v1] regress: remove unnecessary subquery around
current_database()
Replace uses of (SELECT current_database()) with current_database()
in stats.sql and the corresponding expected output.
Author: Chao Li <[email protected]>
---
src/test/regress/expected/stats.out | 12 ++++++------
src/test/regress/sql/stats.sql | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 981d7c90822..0917d2c1266 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -931,7 +931,7 @@ SELECT (n_tup_ins + n_tup_upd) > 0 AS has_data FROM pg_stat_all_tables
-- Test that various stats views are being properly populated
-----
-- Test that sessions is incremented when a new session is started in pg_stat_database
-SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = current_database() \gset
\c
SELECT pg_stat_force_next_flush();
pg_stat_force_next_flush
@@ -939,7 +939,7 @@ SELECT pg_stat_force_next_flush();
(1 row)
-SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = current_database();
?column?
----------
t
@@ -1141,8 +1141,8 @@ SELECT pg_stat_reset();
(1 row)
-SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
-SELECT stats_reset AS dbc_reset_ts FROM pg_stat_database_conflicts WHERE datname = (SELECT current_database()) \gset
+SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname = current_database() \gset
+SELECT stats_reset AS dbc_reset_ts FROM pg_stat_database_conflicts WHERE datname = current_database() \gset
SELECT :'db_reset_ts'::timestamptz = :'dbc_reset_ts'::timestamptz;
?column?
----------
@@ -1155,13 +1155,13 @@ SELECT pg_stat_reset();
(1 row)
-SELECT stats_reset > :'db_reset_ts'::timestamptz FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT stats_reset > :'db_reset_ts'::timestamptz FROM pg_stat_database WHERE datname = current_database();
?column?
----------
t
(1 row)
-SELECT stats_reset > :'dbc_reset_ts'::timestamptz FROM pg_stat_database_conflicts WHERE datname = (SELECT current_database());
+SELECT stats_reset > :'dbc_reset_ts'::timestamptz FROM pg_stat_database_conflicts WHERE datname = current_database();
?column?
----------
t
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 70af96f739f..f782f5f80b5 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -432,10 +432,10 @@ SELECT (n_tup_ins + n_tup_upd) > 0 AS has_data FROM pg_stat_all_tables
-----
-- Test that sessions is incremented when a new session is started in pg_stat_database
-SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
+SELECT sessions AS db_stat_sessions FROM pg_stat_database WHERE datname = current_database() \gset
\c
SELECT pg_stat_force_next_flush();
-SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
+SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = current_database();
-- Test pg_stat_checkpointer checkpointer-related stats, together with pg_stat_wal
SELECT num_requested AS rqst_ckpts_before FROM pg_stat_checkpointer \gset
@@ -529,12 +529,12 @@ SELECT pg_stat_reset_shared('unknown');
-- have a baseline for comparison. The same for pg_stat_database_conflicts as it shares
-- the same stats_reset as pg_stat_database.
SELECT pg_stat_reset();
-SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname = (SELECT current_database()) \gset
-SELECT stats_reset AS dbc_reset_ts FROM pg_stat_database_conflicts WHERE datname = (SELECT current_database()) \gset
+SELECT stats_reset AS db_reset_ts FROM pg_stat_database WHERE datname = current_database() \gset
+SELECT stats_reset AS dbc_reset_ts FROM pg_stat_database_conflicts WHERE datname = current_database() \gset
SELECT :'db_reset_ts'::timestamptz = :'dbc_reset_ts'::timestamptz;
SELECT pg_stat_reset();
-SELECT stats_reset > :'db_reset_ts'::timestamptz FROM pg_stat_database WHERE datname = (SELECT current_database());
-SELECT stats_reset > :'dbc_reset_ts'::timestamptz FROM pg_stat_database_conflicts WHERE datname = (SELECT current_database());
+SELECT stats_reset > :'db_reset_ts'::timestamptz FROM pg_stat_database WHERE datname = current_database();
+SELECT stats_reset > :'dbc_reset_ts'::timestamptz FROM pg_stat_database_conflicts WHERE datname = current_database();
----
--
2.50.1 (Apple Git-155)
view thread (24+ 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], [email protected], [email protected]
Subject: Re: Add missing stats_reset column to pg_stat_database_conflicts view
In-Reply-To: <[email protected]>
* 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