Re: Remove nonmeaningful prefixes in PgStat_* fields

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Remove nonmeaningful prefixes in PgStat_* fields
Date: 2023-03-22 18:52:23
Message-ID: 20230322185223.gxppwjajvvbtx7m6@liskov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 20, 2023 at 10:05:21AM +0100, Drouvot, Bertrand wrote:
> Hi,
>
> On 3/20/23 8:32 AM, Michael Paquier wrote:
> >
> > /* Total time previously charged to function, as of function start */
> > - instr_time save_f_total_time;
> > + instr_time save_total_time;
> > I have something to say about this one, though.. I find this change a
> > bit confusing. It may be better kept as it is, or I think that we'd
> > better rename also "save_total" and "start" to reflect in a better way
> > what they do, because removing "f_" reduces the meaning of the field
> > with the two others in the same structure.
>
> Thanks for looking at it!
>
> Good point and keeping it as it is currently would not
> affect the work that is/will be done in [1].
>
> So, please find attached V2 attached taking this comment into account.

> diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
> index 35c6d46555..4f21fb2dc2 100644
> --- a/src/backend/utils/adt/pgstatfuncs.c
> +++ b/src/backend/utils/adt/pgstatfuncs.c
> @@ -1552,7 +1552,7 @@ pg_stat_get_xact_tuples_inserted(PG_FUNCTION_ARGS)
> result = 0;
> else
> {
> - result = tabentry->t_counts.t_tuples_inserted;
> + result = tabentry->counts.tuples_inserted;

This comment still has the t_ prefix as does the one for tuples_updated
and deleted.

otherwise, LGTM.

> /* live subtransactions' counts aren't in t_tuples_inserted yet */
> for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
> result += trans->tuples_inserted;
> @@ -1573,7 +1573,7 @@ pg_stat_get_xact_tuples_updated(PG_FUNCTION_ARGS)
> result = 0;
> else
> {
> - result = tabentry->t_counts.t_tuples_updated;
> + result = tabentry->counts.tuples_updated;
> /* live subtransactions' counts aren't in t_tuples_updated yet */
> for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
> result += trans->tuples_updated;
> @@ -1594,7 +1594,7 @@ pg_stat_get_xact_tuples_deleted(PG_FUNCTION_ARGS)
> result = 0;
> else
> {
> - result = tabentry->t_counts.t_tuples_deleted;
> + result = tabentry->counts.tuples_deleted;
> /* live subtransactions' counts aren't in t_tuples_deleted yet */
> for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
> result += trans->tuples_deleted;
> @@ -1613,7 +1613,7 @@ pg_stat_get_xact_tuples_hot_updated(PG_FUNCTION_ARGS)
> if ((tabentry = find_tabstat_entry(relid)) == NULL)
> result = 0;
> else
> - result = (int64) (tabentry->t_counts.t_tuples_hot_updated);
> + result = (int64) (tabentry->counts.tuples_hot_updated);
>
> PG_RETURN_INT64(result);
> }

- Melanie

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-03-22 18:59:17 Re: meson documentation build open issues
Previous Message Tom Lane 2023-03-22 18:42:26 Re: Request for comment on setting binary format output per session