Dear Sergey & Hackers,
+1 to the idea, and hope it becomes available in v18.
Here are some observations from my review:
1. backend_status.c, Line 572 - the following new condition can break the logic of existing code, please move it inside the IF body, where you compute values for pg_stat_session.
beentry->st_backendType == B_BACKEND
2. Macros PGSTAT_IS_* do not seem to help a lot in code readability, I prefer the original code
3. backend_status.h, Line 193 - Can st_session be defined as a pointer to PgBackendSessionStatus? (Similar to st_sslstatus and st_gssstatus)?
This helps in "shallow" copying PgBackendStatus.
4. backend_status.h, Lines 93-101 - Can these fields be defined as uint64 rather than int64, since they are unsigned in reality?
5. Changes to pg_proc.dat - Andrey Borodin in this video at 46:05 (
https://youtu.be/vTV8XhWf3mo?t=2765) points out that it should be changed by committers only; otherwise frequent conflicts happen.
6. pgstatfuncs.c, line 333: The following line is defined inside the function pg_stat_get_session, but I think it's better suited for pgstatfuncs.h (though existing function - pg_stat_get_progress_info - does the same)
#define PG_STAT_GET_SESSION_COLS 9
7. Some information is currently scattered between pg_stat_session and pg_stat_activity, making joins necessary. However, pg_stat_activity contains two types of columns: Those being immutable during a session (e.g., client_addr, application_name, usename) and those being volatile during the session (e.g., query, query_start, state). The following is thus a "careless" join, combining immutable, volatile, and cumulative info together:
select * from pg_stat_activity join pg_stat_session using (pid);
I suggest adding all or some of the immutable columns of pg_stat_activity to pg_stat_session, thus excluding the need to join the two views (for all practical purposes).
Best Regards,
Sadeq Dousti
