Re: Generate pg_stat_get_xact*() functions with Macros - Mailing list pgsql-hackers

From Corey Huinker
Subject Re: Generate pg_stat_get_xact*() functions with Macros
Date
Msg-id CADkLM=fYtDcB+PRaR_Gg4WbzE6rEL1ZsHfkB0Q5CM22+XR2mJw@mail.gmail.com
Whole thread Raw
In response to Generate pg_stat_get_xact*() functions with Macros  ("Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>)
Responses Re: Generate pg_stat_get_xact*() functions with Macros
Re: Generate pg_stat_get_xact*() functions with Macros
List pgsql-hackers


On Thu, Jan 5, 2023 at 8:50 AM Drouvot, Bertrand <bertranddrouvot.pg@gmail.com> wrote:
Hi hackers,

Please find attached a patch proposal to $SUBJECT.

This is the same kind of work that has been done in 83a1a1b566 and 8018ffbf58 but this time for the
pg_stat_get_xact*() functions (as suggested by Andres in [1]).

The function names remain the same, but some fields have to be renamed.

While at it, I also took the opportunity to create the macros for pg_stat_get_xact_function_total_time(),
pg_stat_get_xact_function_self_time() and pg_stat_get_function_total_time(), pg_stat_get_function_self_time()
(even if the same code pattern is only repeated two 2 times).

Now that this patch renames some fields, I think that, for consistency, those ones should be renamed too (aka remove the f_ and t_ prefixes):

PgStat_FunctionCounts.f_numcalls
PgStat_StatFuncEntry.f_numcalls
PgStat_TableCounts.t_truncdropped
PgStat_TableCounts.t_delta_live_tuples
PgStat_TableCounts.t_delta_dead_tuples
PgStat_TableCounts.t_changed_tuples

But I think it would be better to do it in a follow-up patch (once this one get committed).

[1]: https://www.postgresql.org/message-id/20230105002733.ealhzubjaiqis6ua%40awork3.anarazel.de

Looking forward to your feedback,

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

I like code cleanups like this. It makes sense, it results in less code, and anyone doing a `git grep pg_stat_get_live_tuples` will quickly find the macro definition.

Unsurprisingly, it passes `make check-world`.

So I think it's good to go as-is.

It does get me wondering, however, if we reordered the three typedefs to group like-typed registers together, we could make them an array with the names becoming defined constant index values (or keeping them via a union), then the typedefs effectively become:

typedef struct PgStat_FunctionCallUsage
{
    PgStat_FunctionCounts *fs;
    instr_time  time_counters[3];
} PgStat_FunctionCallUsage;

typedef struct PgStat_BackendSubEntry
{
    PgStat_Counter counters[2];
} PgStat_BackendSubEntry;

typedef struct PgStat_TableCounts
{
    bool        t_truncdropped;
    PgStat_Counter counters[12];
} PgStat_TableCounts;

Then we'd only have 3 actual C functions:

pg_stat_get_xact_counter(oid, int)
pg_stat_get_xact_subtrans_counter(oid, int)
pg_stat_get_xact_function_time_counter(oid, int)

and then the existing functions become SQL standard function body calls, something like this:

CREATE OR REPLACE FUNCTION pg_stat_get_xact_numscans(oid)
 RETURNS bigint
 LANGUAGE sql
 STABLE PARALLEL RESTRICTED COST 1
RETURN pg_stat_get_xact_counter($1, 0);

CREATE OR REPLACE FUNCTION pg_stat_get_xact_tuples_returned(oid)
 RETURNS bigint
 LANGUAGE sql
 STABLE PARALLEL RESTRICTED COST 1
RETURN pg_stat_get_xact_counter($1, 1);


The most obvious drawback to this approach is that the C functions would need to do runtime bounds checking on the index parameter, and the amount of memory footprint saved by going from 17 short functions to 3 is not enough to make any real difference. So I think your approach is better, but I wanted to throw this idea out there.

pgsql-hackers by date:

Previous
From: Justin Pryzby
Date:
Subject: Re: Add tracking of backend memory allocated to pg_stat_activity
Next
From: Tomas Vondra
Date:
Subject: Re: postgres_fdw: using TABLESAMPLE to collect remote sample