Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 10 Dec 2025 17:04:16 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
Message-ID: <aTkpgP9Xe5QJ5sDu@paquier.xyz>
References: <aTZxqzPWr0wMVLq2@paquier.xyz>
 <CAA5RZ0tST1=piXSQwRn2y+YB-VwS+Dtr0Wk27Dw7ED+Xp6qSXQ@mail.gmail.com>
 <A5D8954F-2E20-4BF5-BA0E-79FDC06A632F@gmail.com>
 <CAA5RZ0sg6gskHRgfymo9njEWuXDyn0Zwe+0bcX=nByyE7W+6bw@mail.gmail.com>
 <aTepXZ97PsXpuywI@paquier.xyz>
 <BF27BA43-D94D-4DC6-BD91-95EEAD42AD0F@gmail.com>
 <9252DA7C-C611-4A93-A484-E6CD42557C15@gmail.com>
 <CAA5RZ0vQvmatp8VAk6zroyeKDCw+QVq4XmoRNAFogQo6bJEbNQ@mail.gmail.com>
 <4E61FDC4-CFBB-4116-980F-75ADD245A1F6@gmail.com>
 <CAA5RZ0swDCK+7M2KQip0tqUsBK3LUHd3F0ytpsKdDCfygHksKg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="su3aFBg80R6gXPbe"
Content-Disposition: inline
In-Reply-To: 
 <CAA5RZ0swDCK+7M2KQip0tqUsBK3LUHd3F0ytpsKdDCfygHksKg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aTkpgP9Xe5QJ5sDu%40paquier.xyz>
Precedence: bulk


--su3aFBg80R6gXPbe
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 09, 2025 at 05:15:45PM -0600, Sami Imseih wrote:
> Thanks for explaining. If there is a good use-case to add more detail to
> the =E2=80=9Cend=E2=80=9D callback, it=E2=80=99s not very obvious yet. Ma=
ybe in the future, there
> will be a convincing reason to do so.

The step taken by test_custom_var_stats_file_cleanup() for the
STATS_READ case shines for its simplicity.  The STATS_DISCARD case is
also simple: we know that we want to ditch the stats.

Now, it is kind of true that the STATS_WRITE case feels a bit
disturbing written this way: we let a module take an action, but we
don't actually know the state of the main pgstats file when inside the
callback.  I mean, you can know how things are going on, but it means
that a module can just rely on a check if
PGSTAT_STAT_PERMANENT_FILENAME is on disk, but an unlink() could have
failed as well.  So, yes, I am wondering whether we should do what
Chao is suggesting, passing an extra state to the callback to let the
module know if we have actually succeeded or failed the operations
that have been taken on the main stats file before the callback
end_extra_stats is called in the three cases.  It does not matter for
the STATS_READ case, but it may matter for the STATS_DISCARD or
STATS_WRITE case.

> When we hit the clean-up code on any =E2=80=9Cerror=E2=80=9D, it should b=
e accompanied by
> an error log. That is
> done in all cases inside pgstat.c, and I expect an extension to log the
> error as well.

FWIW, I still have the same question as the one posted here about the
business in pgstat_initialize(), still present in v6:
https://www.postgresql.org/message-id/aTepXZ97PsXpuywI@paquier.xyz

This remains unanswered.
--
Michael

--su3aFBg80R6gXPbe
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmk5KYAACgkQnvQgOdby
QH0S/Q//RtmmLire7rNyKWU18nTzoLN8YWe00iZNbd0wH9m0exA6fc1wimOyyFLi
DoSjdfwyT042kMn1Jw+52uaGl+iXVPDIztZzDnYX1YHEwu7tZtocrcrhF1Tu9zhJ
lZQGho2fIJh9UVC5s0SQjArvX0lBpu7G46V5qUtXtic3aSuTwbbRCondjtkMASXk
Kasp0tqJeQnFU4wa+ZKgubm8iovtpcTDj1BqttmSx5Rs6oMQ0EKh4FaPre9hVJ64
2C1x8+EF2UPigGOBBAcSf4MMDhqI4o1x8LOBpQK5/c9rR0Kne3wpn/dirM4vhH5/
/l1izGTecjY6ToDW6AVTxkxe9n9qMorvR841R3VgiJ4NvY5vD0U5MQeWdSzomErB
+fgw+zeVKbTSxfTOQOJlYBTgVYMxXg7vwd7RzgYWuxZXFzhS+FkHc/yFJknmF9N/
yayhoP0ZOEybR92BsnaHX1kxL1vGXYMEvjH5SoulDfm29lt5KlAoS7h7vO6Pg8Pc
KMEEdJYAI07IanHj/zG9ex5xS5vko1smIE8RQIzl9bWHUjAJIGXaOJolHwRkNkqA
cBiQ3u8fzBbMulphqnk8s6/WX52pfMPLlxLPl6uAc+6ZadCNiFdnvnD4vFnMo1wW
fTIknfyicH7d0Rt9SGd7f/TEbM2nNkm6hb8gPHfdHmfZ935lxlg=
=ajrk
-----END PGP SIGNATURE-----

--su3aFBg80R6gXPbe--