Re: [Proposal] Adding callback support for custom statistics kinds
| От | Sami Imseih |
|---|---|
| Тема | Re: [Proposal] Adding callback support for custom statistics kinds |
| Дата | |
| Msg-id | CAA5RZ0t-WDUhjkpdnfxH_OZWJAKtozQSV+nv3+H4ft4+FxB1Yw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [Proposal] Adding callback support for custom statistics kinds (Michael Paquier <michael@paquier.xyz>) |
| Ответы |
Re: [Proposal] Adding callback support for custom statistics kinds
|
| Список | pgsql-hackers |
> On Wed, Dec 17, 2025 at 08:03:36AM +0100, Peter Eisentraut wrote: > > So it seems to me that either the callbacks API needs some adjustments, or > > this particular implementation of the callback function is incorrect. > > Hmm, you are right that this is not aligned. This can be improved > with one change for each callback: > - It is OK with from_serialized_data() to manipulate the header data, > because we want to fill a portion of the shmem data with extra data > read from disk (the module wants to add a reference to a DSA stored in > the shmem entry, read from the second file). So we should discard the > const marker from the callback definition. > - The const usage is OK for to_serialized_data(): it is better to > encourage a policy where the header data cannot be manipulated. So > the const needs to be kept in the definition, but I also think that we > should change the module implementation so as the cast to > PgStatShared_CustomVarEntry is a const. > > These changes result in the attached. Sami, what do you think? I agree. This was a miss during the review. Thanks for raising this. The fix looks correct to me in which the from_serialized_data callback is expected to modify the header, to reconstruct the entry and the to_serialized_data is never expected to modify the header, since we are only reading what is currently in stats. I can't think of a reason to ever have to modify the entry while writing out to disk. I got the attached patch ready with some additional comments in the callback definitions to clarify the API contract. We only need to call out the "header' nuance since it's a const in one callback and not the other. "key" is self documenting being a const in both cases. -- Sami Imseih Amazon Web Services (AWS)
Вложения
В списке pgsql-hackers по дате отправления: