Re: Add support for entry counting in pgstats

> On Sep 12, 2025, at 15:23, Michael Paquier <michael@paquier.xyz> wrote:
> 
> 
> --
> Michael
> <0001-Add-support-for-entry-counting-in-pgstats.patch><0002-injection_points-Add-entry-counting.patch>


The code overall looks good to me, very clear and neat. Just a few small comments:

1 - 0001
```
+	 * set.  This counter is incremented each time a new entry is created (not
+	 * when reused), and each time an entry is dropped.
+	 */
+	pg_atomic_uint64 entry_counts[PGSTAT_KIND_MAX];
```

“and each time an entry is dropped” is a little misleading, it should be “and decremented when an entry is removed”.

2 - 0001
```
+	/* Increment entry count, if required. */
+	if (kind_info->track_counts)
+		pg_atomic_fetch_add_u64(&pgStatLocal.shmem->entry_counts[kind - 1], 1);
```

The code is quite straightforward, so the comment seems unnecessary.

3 - 0001
```
+		if (kind_info->track_counts)
+		{
+			ereport(ERROR,
+					(errmsg("custom cumulative statistics property is invalid"),
+					 errhint("Custom cumulative statistics cannot use counter tracking for fixed-numbered objects.")));
+		}
```

{} are not needed. Looks like in the existing code, when “if” has a single statement, {} are not used. There is a similar “if” right above this change.

4 - 0004
```
diff --git a/src/test/modules/injection_points/injection_stats.c b/src/test/modules/injection_points/injection_stats.c
index e3947b23ba57..15ad9883dedb 100644
--- a/src/test/modules/injection_points/injection_stats.c
+++ b/src/test/modules/injection_points/injection_stats.c
@@ -49,6 +49,7 @@ static const PgStat_KindInfo injection_stats = {
 	.shared_data_len = sizeof(((PgStatShared_InjectionPoint *) 0)->stats),
 	.pending_size = sizeof(PgStat_StatInjEntry),
 	.flush_pending_cb = injection_stats_flush_cb,
+	.track_counts = true,
 };
```

Would it be better to move “.track_counts” up to be with the other boolean flags? Whey define together to share the same byte, feels better to initialize them together as well.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/