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 bigintLANGUAGE sqlSTABLE PARALLEL RESTRICTED COST 1RETURN pg_stat_get_xact_counter($1, 0);
CREATE OR REPLACE FUNCTION pg_stat_get_xact_tuples_returned(oid)RETURNS bigintLANGUAGE sqlSTABLE PARALLEL RESTRICTED COST 1RETURN 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: