public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Michael Paquier <[email protected]>
To: Sami Imseih <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Date: Thu, 9 Jul 2026 11:10:18 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAA5RZ0s9SDOu+Z6veoJCHWk+kDeTktAtC-KY9fQ9Z6BJdDUirQ@mail.gmail.com>
<[email protected]>
<CAA5RZ0ux07=E97mJ3JCy2jhs349NKzj7SPJNbDWZ11m9BDkTQQ@mail.gmail.com>
<CAA5RZ0tnopAS4MCcoxXdJShYoGxYZgsoDXCjcie0Q-2TBPuJwA@mail.gmail.com>
<[email protected]>
<CAA5RZ0skT-uWwgrCg3Hj3AQaBbQdN0h72scD9GBLxH1nEKSNwQ@mail.gmail.com>
<CAA5RZ0tH6ih3anQ=Lcwttm+8BURJ8VkBoy_9V8Ty73ZA1r384Q@mail.gmail.com>
<[email protected]>
<CAA5RZ0tvDvHYYLnu-T0DRK0xv1qv2_EK1yHjth5uAqDWz_GvfA@mail.gmail.com>
<CAA5RZ0thcqk_r_SCB3TdXP9hVV5mG_29yc9GaGr4ujPQmcLDEQ@mail.gmail.com>
<[email protected]>
On 25.11.25 03:03, Michael Paquier wrote:
> One part of the proposed patch that felt independent to me was the
> renaming and publishing of the two write/read routines for the stats
> files, so I have extracted that in your first patch to reduce the
> blast, and applied that as it can also be useful on its own.
I came across this now. I'm concerned that these write/read routines
for stats files are a bit weirdly defined.
They were originally internal to pgstat.c where they were specifically
tailored to the control flow in that file. For example,
pgstat_write_chunk() contains an internal comment "We check for errors
with ferror() when done writing the stats.", which is fine as a
description of what happens in that file, but it's pretty poor as an API
documentation. On the other hand, pgstat_read_chunk() does do some
error checking, but completely undocumented and weirdly inconsistent
with the write counterpart. Again, this might be fine for an internal
helper but not for an external API.
I also don't think these functions are well-named. They don't do
anything about "pgstat", they are just convenience wrappers around
fread() and fwrite() with some sizeof integration (which is also kind of
fragile, because there is no type checking about what you are passing).
I have not found any documentation about this new feature for pluggable
statistics, so it's not clear whether these functions are meant to be
part of the official API for that, or they just happened to be available
for implementing that test module.
I suggest that unless we can come up with a well-defined, robust, and
well-documented API for these, they should be moved back to be internal
to pgstat.c, and test_custom_var_stats.c can define its own helper
functions. Maybe in the future, when we have more of these, some good
API will emerge.
view thread (47+ messages)
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]
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