Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <BF27BA43-D94D-4DC6-BD91-95EEAD42AD0F@gmail.com>
Date: Tue, 9 Dec 2025 13:35:47 +0800
Cc: Sami Imseih <samimseih@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <9252DA7C-C611-4A93-A484-E6CD42557C15@gmail.com>
References: <4FEEC2B8-D9CF-4BD0-AA7E-44CE3E27ADD8@gmail.com>
 <aS_QrGCgB7AMlSdU@paquier.xyz>
 <5EC13E39-B41E-464A-B382-0ACEB18A15BB@gmail.com>
 <CAA5RZ0sJgO6GAwgFxmzg9MVP=rM7Us8KKcWpuqxe-f5qxmpE0g@mail.gmail.com>
 <aTIV8qfcIkht1Hgv@paquier.xyz>
 <CAA5RZ0sG2RUKg=OLY+6-e4q=X9rsLfK3pKn03d=RZQppEDR=Bg@mail.gmail.com>
 <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>
To: Michael Paquier <michael@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/9252DA7C-C611-4A93-A484-E6CD42557C15%40gmail.com>
Precedence: bulk


> On Dec 9, 2025, at 13:23, Chao Li <li.evan.chao@gmail.com> wrote:
>=20
>=20
>=20
>> On Dec 9, 2025, at 12:45, Michael Paquier <michael@paquier.xyz> =
wrote:
>>=20
>> It seems to me that you are missing nothing here, and that Chao has
>> missed the fact that the end of pgstat_read_statsfile() does a "goto
>> done", meaning that we would take a round of
>=20
> No, I didn=E2=80=99t miss that part. But in the =E2=80=9Cdone=E2=80=9D =
clause:
>=20
> ```
> done:
> /* First, cleanup the main stats file, PGSTAT_STAT_PERMANENT_FILENAME =
*/
> FreeFile(fpin);
>=20
> elog(DEBUG2, "removing permanent stats file \"%s\"", statfile);
> unlink(statfile);
>=20
> /* Let each stats kind run its cleanup callback, if it provides one */
> for (PgStat_Kind kind =3D PGSTAT_KIND_MIN; kind <=3D PGSTAT_KIND_MAX; =
kind++)
> {
> const PgStat_KindInfo *kind_info =3D pgstat_get_kind_info(kind);
>=20
> if (kind_info && kind_info->end_extra_stats)
> kind_info->end_extra_stats(STATS_READ);
> }
> ```
>=20
> end_extra_stats(STATS_READ) has no failure indication.
>=20

Sorry, I incidentally clicked =E2=80=9Csend=E2=80=9D too quickly.

My point is that, there are many places jumping to =E2=80=9Cerror=E2=80=9D=
, then from =E2=80=9Cerror=E2=80=9D goto =E2=80=9Cdone=E2=80=9D, if an =
error didn=E2=80=99t happen from the deserialize callback, how =
end_extra_stats() can know if failure happened and takes action =
accordingly?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/