Re: Add connection active, idle time to pg_stat_activity

>
> 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