Sawada-san, Thank you your comments. The attached patch reflects the comment. I also made a fix for the regression test.
Regards, Noriyoshi Shinoda -----Original Message----- From: Masahiko Sawada [mailto:masahiko.saw...@2ndquadrant.com] Sent: Monday, October 12, 2020 8:12 PM To: Shinoda, Noriyoshi (PN Japan A&PS Delivery) <noriyoshi.shin...@hpe.com> Cc: Amit Kapila <amit.kapil...@gmail.com>; Dilip Kumar <dilipbal...@gmail.com>; Magnus Hagander <mag...@hagander.net>; Tomas Vondra <tomas.von...@2ndquadrant.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Ajin Cherian <itsa...@gmail.com> Subject: Re: Resetting spilled txn statistics in pg_stat_replication On Mon, 12 Oct 2020 at 18:29, Shinoda, Noriyoshi (PN Japan A&PS Delivery) <noriyoshi.shin...@hpe.com> wrote: > > Hi, thank you for the awesome feature. > Thank you for reporting! > As it may have been discussed, I think the 'name' column in > pg_stat_replication_slots is more consistent with the column name and data > type matched to the pg_replication_slots catalog. > The attached patch changes the name and data type of the 'name' column to > slot_name and 'name' type, respectively. It seems a good idea to me. In other system views, we use the name data type for object name. When I wrote the first patch, I borrowed the code for pg_stat_slru which uses text data for the name but I think it's an oversight. > Also, the macro name PG_STAT_GET_..._CLOS has been changed to > PG_STAT_GET_..._COLS. Good catch! Here is my comments on the patch: --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -798,7 +798,7 @@ CREATE VIEW pg_stat_replication AS CREATE VIEW pg_stat_replication_slots AS SELECT - s.name, + s.name AS slot_name, s.spill_txns, s.spill_count, s.spill_bytes, I think we should modify 'proargnames' of pg_stat_get_replication_slots() in pg_proc.dat instead. --- --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -7094,7 +7094,7 @@ pgstat_replslot_index(const char *name, bool create_it) Assert(nReplSlotStats <= max_replication_slots); for (i = 0; i < nReplSlotStats; i++) { - if (strcmp(replSlotStats[i].slotname, name) == 0) + if (strcmp(replSlotStats[i].slotname.data, name) == 0) return i; /* found */ } @@ -7107,7 +7107,7 @@ pgstat_replslot_index(const char *name, bool create_it) /* Register new slot */ memset(&replSlotStats[nReplSlotStats], 0, sizeof(PgStat_ReplSlotStats)); - memcpy(&replSlotStats[nReplSlotStats].slotname, name, NAMEDATALEN); + memcpy(&replSlotStats[nReplSlotStats].slotname.data, name, + NAMEDATALEN); return nReplSlotStats++; } We can use NameStr() instead. --- Perhaps we need to update the regression test as well. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pg_stat_replication_slots_v2.diff
Description: pg_stat_replication_slots_v2.diff
