public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Drouvot, Bertrand <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Generate pg_stat_get_xact*() functions with Macros
Date: Thu, 12 Jan 2023 10:24:34 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>

Hi,

On 2023-01-12 08:38:57 +0100, Drouvot, Bertrand wrote:
> On 1/11/23 11:59 PM, Andres Freund wrote:
> > > Now that this patch renames some fields
> > 
> > I don't mind renaming the fields - the prefixes really don't provide anything
> > useful. But it's not clear why this is related to this patch? You could just
> > include the f_ prefix in the macro, no?
> > 
> > 
> 
> Right, but the idea is to take the same approach that the one used in 8018ffbf58 (where placing the prefixes in the macro
> would have been possible too).

I'm not super happy about that patch tbh.


> > Probably should remove PgStat_BackendFunctionEntry.
> 
> I think that would be a 3rd patch, agree?

Yep.



> > I now see that PG_STAT_GET_DBENTRY_FLOAT8 already exists, defined the same
> > way. But the name fields misleading enough that I'd be inclined to rename it?
> > 
> 
> PG_STAT_GET_FUNCENTRY_FLOAT8_MS looks good by me. Waiting on what we'll decide for
> the existing PG_STAT_GET_DBENTRY_FLOAT8 (so that I can align for the PG_STAT_GET_FUNCENTRY_FLOAT8).

+1



> > Although I suspect this actually hints at an architectural thing that could be
> > fixed better: Perhaps we should replace find_tabstat_entry() with a version
> > returning a fully "reconciled" PgStat_StatTabEntry?  It feels quite wrong to
> > have that intimitate knowledge of the subtransaction stuff in pgstatfuncs.c
> > and about how the different counts get combined.
> > 
> > I think that'd allow us to move the definition of PgStat_TableStatus to
> > PgStat_TableXactStatus, PgStat_TableCounts to pgstat_internal.h. Which feels a
> > heck of a lot cleaner.
> 
> Yeah, I think that would be for a 4th patch, agree?

Yea. I am of multiple minds about the ordering. I can see benefits on fixing
the architectural issue before reducing duplication in the accessor with a
macro. The reason is that if we addressed the architectural issue, the
difference between the xact and non-xact version will be very minimal, and
could even be generated by the same macro.

Greetings,

Andres Freund

view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Generate pg_stat_get_xact*() functions with Macros
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox