MIME-Version: 1.0
References: 
 <CAA5RZ0tnopAS4MCcoxXdJShYoGxYZgsoDXCjcie0Q-2TBPuJwA@mail.gmail.com>
 <aRJzdl_cSc3XHX85@paquier.xyz>
 <CAA5RZ0skT-uWwgrCg3Hj3AQaBbQdN0h72scD9GBLxH1nEKSNwQ@mail.gmail.com>
 <CAA5RZ0tH6ih3anQ=Lcwttm+8BURJ8VkBoy_9V8Ty73ZA1r384Q@mail.gmail.com>
 <aSOidfDnBT4lbtHp@paquier.xyz>
 <CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@mail.gmail.com>
 <CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
 <aSUOaqT53UBIYTKh@paquier.xyz>
 <CAA5RZ0ufds2r3H7QUro_yJDrnUmsQ4ocufqgcsRGoZvxQKeMrA@mail.gmail.com>
 <4FEEC2B8-D9CF-4BD0-AA7E-44CE3E27ADD8@gmail.com>
 <aS_QrGCgB7AMlSdU@paquier.xyz>
 <5EC13E39-B41E-464A-B382-0ACEB18A15BB@gmail.com>
In-Reply-To: <5EC13E39-B41E-464A-B382-0ACEB18A15BB@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
Date: Thu, 4 Dec 2025 17:00:10 -0600
Message-ID: 
 <CAA5RZ0sJgO6GAwgFxmzg9MVP=rM7Us8KKcWpuqxe-f5qxmpE0g@mail.gmail.com>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
To: Chao Li <li.evan.chao@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA5RZ0sJgO6GAwgFxmzg9MVP%3DrM7Us8KKcWpuqxe-f5qxmpE0g%40mail.gmail.com>
Precedence: bulk

> If we push down the responsibility into the extension code, then all extensions
> that want to enjoy the callbacks have to handle the same complexities of dealing
> with stat files, which sounds big duplicate efforts.

Thanks for the input! Yes, this is a trade-off between putting
responsibility on the
extension vs core. The initial thought I had was exactly like yours, but it will
be easier to get something pushed if we make the core changes as minimal as
possible. If there are enough complaints in the future, this can be revisited.
Particularly if there is a common patterns for file cleanup, this
could be turned
into a core utility.

> That looks like a good direction to me.  The only differences I can
> see with the stats module in injection_points for variable-sized stats
> is that this new module does not check pgstat_drop_entry() and
> pgstat_fetch_entry() when working on a custom stats kind.  If we had
> SQL interfaces calling these two, we could just remove
> injection_stats.c entirely, moving everything to this new test module.

> I should have invented a new module from the start, perhaps, but well,
> that was good enough to check the basic APIs when working on the
> custom APIs.  Removing this duplication would be my own business with
> your module in the tree, no need for you to worry about that.  That
> would also remove the tweak you have used regarding the duplicated
> kind ID.

I plan on addressing the other comments.

However, as discussed off-list, I do think moving the custom kind tests from
injection points to the new test module is a prerequisite. I rather
not  have us
push a new test module that is doing duplicate work as the injection
stats tests.
I worked on this refactoring today and plan to have a patch ready for review
by tomorrow.

--
Sami Imseih
Amazon Web Services (AWS)