> > I've rebased the patch according to the CI bot's requirements. Please have > a look. >
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
