Hi,

On 2022-11-15 10:48:40 +0100, Drouvot, Bertrand wrote:
> diff --git a/src/backend/utils/adt/pgstatfuncs.c 
> b/src/backend/utils/adt/pgstatfuncs.c
> index ae3365d917..be7f175bf1 100644
> --- a/src/backend/utils/adt/pgstatfuncs.c
> +++ b/src/backend/utils/adt/pgstatfuncs.c
> @@ -36,24 +36,34 @@
>  
>  #define HAS_PGSTAT_PERMISSIONS(role)  (has_privs_of_role(GetUserId(), 
> ROLE_PG_READ_ALL_STATS) || has_privs_of_role(GetUserId(), role))
>  
> +#define PGSTAT_FETCH_STAT_ENTRY(entry, stat_name) ((entry == NULL) ? 0 : 
> (int64) (entry->stat_name))
> +
>  Datum
> -pg_stat_get_numscans(PG_FUNCTION_ARGS)
> +pg_stat_get_index_numscans(PG_FUNCTION_ARGS)
>  {
>       Oid                     relid = PG_GETARG_OID(0);
>       int64           result;
> -     PgStat_StatTabEntry *tabentry;
> +     PgStat_StatIndEntry *indentry = pgstat_fetch_stat_indentry(relid);
>  
> -     if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
> -             result = 0;
> -     else
> -             result = (int64) (tabentry->numscans);
> +     result = PGSTAT_FETCH_STAT_ENTRY(indentry, numscans);
>  
>       PG_RETURN_INT64(result);
>  }

This still leaves a fair bit of boilerplate. ISTM that the function body
really should just be a single line.

Might even be worth defining the whole function via a macro. Perhaps something 
like

PGSTAT_DEFINE_REL_FIELD_ACCESSOR(PGSTAT_KIND_INDEX, pg_stat_get_index, 
numscans);

I think the logic to infer which DB oid to use for a stats entry could be
shared between different kinds of stats. We don't need to duplicate it.

Is there any reason to not replace the "double lookup" in
pgstat_fetch_stat_tabentry() with IsSharedRelation()?


This should probably be done in a preparatory commit.

Greetings,

Andres Freund


Reply via email to