MIME-Version: 1.0
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>
 <9252DA7C-C611-4A93-A484-E6CD42557C15@gmail.com>
 <CAA5RZ0vQvmatp8VAk6zroyeKDCw+QVq4XmoRNAFogQo6bJEbNQ@mail.gmail.com>
 <4E61FDC4-CFBB-4116-980F-75ADD245A1F6@gmail.com>
In-Reply-To: <4E61FDC4-CFBB-4116-980F-75ADD245A1F6@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
Date: Tue, 9 Dec 2025 17:15:45 -0600
Message-ID: 
 <CAA5RZ0swDCK+7M2KQip0tqUsBK3LUHd3F0ytpsKdDCfygHksKg@mail.gmail.com>
Subject: Re: [Proposal] Adding callback support for custom statistics kinds
To: Chao Li <li.evan.chao@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="00000000000010eb0a06458d186b"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA5RZ0swDCK%2B7M2KQip0tqUsBK3LUHd3F0ytpsKdDCfygHksKg%40mail.gmail.com>
Precedence: bulk

--00000000000010eb0a06458d186b
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

>
> > IIUC, if *any* error occurs outside of a deserialize callback, first th=
e
> "error"
> > code will be called, followed by "done" which will then trigger the
> > end_extra_stats
> > callback that will perform the cleanup.
>
> That is true. But problem is, without an error indication,
> end_extra_stats(STATS_READ) can only blindly perform cleanup works. As yo=
u
> are providing general purposed callbacks, who knows what scenarios
> extensions would do, so it=E2=80=99s better to provide more information t=
o
> callbacks. IMO, letting end_extra_stats() know current situation (normal =
or
> failure, even error code) is very meaningful. For example, my extension m=
ay
> want to log =E2=80=9CI am forced to quite due to outside error=E2=80=9D o=
r =E2=80=9CI am done
> successfully=E2=80=9D in end_extra_stats(). Anyway, that=E2=80=99s my own=
 opinion. If you
> and Michael still consider that=E2=80=99s not a problem, I won=E2=80=99t =
argue more.


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. Mayb=
e in the future, there
will be a convincing reason to do so.

When we hit the clean-up code on any =E2=80=9Cerror=E2=80=9D, it should be =
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.


--
Sami Imseih
Amazon Web Services (AWS)

--00000000000010eb0a06458d186b
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div class=3D"gmail_quote gmail_quote_container" dir=3D"auto"><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;=
padding-left:1ex" dir=3D"auto">&gt; IIUC, if *any* error occurs outside of =
a deserialize callback, first the &quot;error&quot;<br>
&gt; code will be called, followed by &quot;done&quot; which will then trig=
ger the<br>
&gt; end_extra_stats<br>
&gt; callback that will perform the cleanup.<br>
<br>
That is true. But problem is, without an error indication, end_extra_stats(=
STATS_READ) can only blindly perform cleanup works. As you are providing ge=
neral purposed callbacks, who knows what scenarios extensions would do, so =
it=E2=80=99s better to provide more information to callbacks. IMO, letting =
end_extra_stats() know current situation (normal or failure, even error cod=
e) is very meaningful. For example, my extension may want to log =E2=80=9CI=
 am forced to quite due to outside error=E2=80=9D or =E2=80=9CI am done suc=
cessfully=E2=80=9D in end_extra_stats(). Anyway, that=E2=80=99s my own opin=
ion. If you and Michael still consider that=E2=80=99s not a problem, I won=
=E2=80=99t argue more.</blockquote><div dir=3D"auto"><br></div><div dir=3D"=
auto">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=C2=A0<span st=
yle=3D"font-family:-apple-system,sans-serif">obvious yet. Maybe in the futu=
re, there will be a convincing reason to do so.=C2=A0</span></div><div dir=
=3D"auto"><span style=3D"font-family:-apple-system,sans-serif"><br></span><=
/div><div dir=3D"auto"><span style=3D"font-family:-apple-system,sans-serif"=
>When we hit the clean-up code on any =E2=80=9Cerror=E2=80=9D, it should be=
 accompanied by an error log. That is=C2=A0</span></div><div dir=3D"auto">d=
one in all cases inside pgstat.c, and I expect an=C2=A0<span style=3D"font-=
family:-apple-system,sans-serif">extension to log the error as well.=C2=A0<=
/span></div><div dir=3D"auto"><br></div><div dir=3D"auto"><div><div style=
=3D"color:rgb(0,0,0);font-family:&#39;-apple-system&#39;,sans-serif;font-si=
ze:16px;font-style:normal;font-weight:400;letter-spacing:normal;text-indent=
:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoratio=
n:none"><br clear=3D"all"><div><div dir=3D"ltr" data-smartmail=3D"gmail_sig=
nature"><div dir=3D"ltr"><div>--</div><div>Sami Imseih</div><div>Amazon Web=
 Services (AWS)</div></div></div></div></div><br></div><br></div><blockquot=
e class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc sol=
id;padding-left:1ex" dir=3D"auto"></blockquote></div>

--00000000000010eb0a06458d186b--