public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Sami Imseih <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Wed, 17 Dec 2025 08:03:36 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAA5RZ0ubfcmkkY7i07CmrkY12H8BsRXDoOutYXjYz0gsq7hm-w@mail.gmail.com>
References: <CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAA5RZ0vQvmatp8VAk6zroyeKDCw+QVq4XmoRNAFogQo6bJEbNQ@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0swDCK+7M2KQip0tqUsBK3LUHd3F0ytpsKdDCfygHksKg@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0uC4j2QGK85ma6bkF6YOWBrYX9J3dV2CEMg3d0CBLpHSA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAA5RZ0ubfcmkkY7i07CmrkY12H8BsRXDoOutYXjYz0gsq7hm-w@mail.gmail.com>

On 13.12.25 01:41, Sami Imseih wrote:
> Thanks for the updates!
> 
>> - Less fwrite() and fread(), more read_chunk() and write_chunk().  We
>> are exposing these APIs, let's use them.
> 
> oops. That totally slipped my mind :( sorry about that.
> 
>> - The callbacks are renamed, to be more generic: "finish" for the
>> end-of-operation actions and to/from_serialized_data.
> 
> At first I wasn’t a fan of the name “finish” for the callback.
> I was thinking of calling it “finish_auxiliary”. But, we’re not
> forcing callbacks to be used together, and there could perhaps
> be cases where “finish" can be used on its own, so this is fine by me.
> 
> I made some changes as well, in v8:
> 
> 1/ looks like b4cbc106a6ce snuck into v7. I fixed that.
> 
> 2/ After looking this over, I realized that “extra” and “auxiliary”
> were being used interchangeably. To avoid confusion, I replaced all
> instances of “extra” with “auxiliary" in both the comments and
> macros, i.e. TEST_CUSTOM_AUX_DATA_DESC

The function test_custom_stats_var_from_serialized_data() takes an 
argument of type

     const PgStatShared_Common *header

which is then later cast

     entry = (PgStatShared_CustomVarEntry *) header;

where entry is defined as

     PgStatShared_CustomVarEntry *entry;

So you are losing the const qualification here.

But fixing that by adding the const qualification to entry would not 
work because what entry points to is later modified:

     entry->description = InvalidDsaPointer;

So the header argument of the function should not be const qualified.

But the signature of that function is apparently determined by this new 
callbacks API, so it cannot be changed in isolation.

So it seems to me that either the callbacks API needs some adjustments, 
or this particular implementation of the callback function is incorrect.

view thread (47+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [Proposal] Adding callback support for custom statistics kinds
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox