Re: Generate pg_stat_get_* functions with Macros

Hi,

On 12/6/22 3:45 AM, Michael Paquier wrote:
> On Mon, Dec 05, 2022 at 05:16:56PM +0900, Michael Paquier wrote:
>> Doing that in a separate patch is fine by me.
>
> I have applied the patch for the tab entries, then could not resist
> poking at the parts for the db entries. This leads to more reduction
> than the other one actually, as of:
> 4 files changed, 169 insertions(+), 447 deletions(-)
>
> Like the previous one, the functions have the same names and the field
> names are updated to fit in the picture. Thoughts?

Thanks! For this one (the INT64 case) the fields renaming are not strictly mandatory as we could add the "n_" in the macro itself, something like:

+#define PG_STAT_GET_DBENTRY_INT64(stat) \
+Datum \
+CppConcat(pg_stat_get_db_,stat)(PG_FUNCTION_ARGS) \
+{ \
+ Oid dbid = PG_GETARG_OID(0); \
+ int64 result; \
+ PgStat_StatDBEntry *dbentry; \
+ \
+ if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL) \
+ result = 0; \
+ else \
+ result = (int64) (dbentry->CppConcat(n_,stat)); \
+ \
+ PG_RETURN_INT64(result); \
+}

Fields renaming was mandatory in the previous ones as there was already a mix of with/without "n_" in the existing fields names.

That said, I think it's better to rename the fields as you did (to be "consistent" on the naming between relation/db stats), so the patch LGTM.

Regards,

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