> 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)
v1-0001-Fix-const-correctness-in-pgstat-serialization-cal.patch
Description: Binary data
