Date: Sat, 26 Feb 2022 14:21:01 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Robert Haas <robertmhaas@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Frontend error logging style
Message-ID: <Yhm4vVGXPaVAT1OE@paquier.xyz>
References: <632f02ed-12ac-b91f-0e99-9c1b086f8cd0@enterprisedb.com>
 <CA+TgmoYBmULn65p4cz3QnL50vJozO=DmO-mKSt9HbVWBMHbgXw@mail.gmail.com>
 <YZMfMI3kTDw/RU0k@paquier.xyz>
 <CA+TgmoZy84orCfUNipEp+QzWwRuR2yrjsdcJN9ROHzPkXDRbxQ@mail.gmail.com>
 <a06f311b-18cd-ee56-ad4c-03ed0f93713f@enterprisedb.com>
 <CA+TgmoYaA1wtU91yNg1_C6oS8azAfp5dQ1YrfuNAm2=Yefrn1A@mail.gmail.com>
 <913687.1645572199@sss.pgh.pa.us>
 <6fdb3ab3-f093-27b9-dfdc-c391e69163fc@enterprisedb.com>
 <1346360.1645715441@sss.pgh.pa.us>
 <1895619.1645809325@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="UW9e6asxQmvxXgYU"
Content-Disposition: inline
In-Reply-To: <1895619.1645809325@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/Yhm4vVGXPaVAT1OE%40paquier.xyz>
Precedence: bulk


--UW9e6asxQmvxXgYU
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Feb 25, 2022 at 12:15:25PM -0500, Tom Lane wrote:
> I feel that the reasonable alternatives are either to drop the FATAL
> log level, or try to make it actually mean something by consistently
> using it for errors that are indeed fatal.  I had a go at doing the
> latter, but eventually concluded that that way madness lies.  It's
> not too hard to use "pg_log_fatal" when there's an exit(1) right
> after it, but there are quite a lot of cases where a subroutine
> reports an error and returns a failure code to its caller, whereupon
> the caller exits.  Either the subroutine has to make an unwarranted
> assumption about what its callers will do, or we need to make an API
> change to allow the subroutine itself to exit(), or we are going to
> present a user experience that is inconsistently different depending
> on internal implementation details.

Nice code cut.

> I conclude that we ought to drop the separate FATAL level and just
> use ERROR instead.

FWIW, I have found the uses of pg_log_error() and pg_log_fatal()
rather confusing in some of the src/bin/ tools, so I am in favor of
removing completely one or the other, and getting rid of FATAL is the
best choice.

> The attached revision does that, standardizes on pg_fatal() as the
> abbreviation for pg_log_error() + exit(1), and invents detail/hint
> features as per previous discussion.

+#define pg_log_warning_hint(...) do { \
+       if (likely(__pg_log_level <= PG_LOG_WARNING)) \
+           pg_log_generic(PG_LOG_WARNING, PG_LOG_HINT, __VA_ARGS__);
\
    } while(0)

Hm.  I am not sure to like much this abstraction by having so many
macros that understand the log level through their name.  Wouldn't it
be cleaner to have only one pg_log_hint() and one pg_log_detail() with
the log level passed as argument of the macro?
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmIZuL0ACgkQnvQgOdby
QH0V9Q//a1zJnP0/et8B/gedsGGBsXUdyMe/eeXB1Wg+I6LbPf/ixGgYfirDAJcG
AhzeBWxzyXSSPmKjBYQ8n2Y7lLQmLVzK9KAwt3PnNsU0zvPuT5iEUZh8dMdcUjkL
aeLE9qbWF1Ozk0lI1ZshA2dYuahJ5YnkrJMQSqsIdtmDUAqyaMiGYQDZrFryq1vb
MYfP8UaYIXg0EaG3vIm9bxUnuTG9fonluzH+J3+//AOXd3jLmdKhkX3SAAuRRxpM
oWJ+H/g2Ce0h/jo3ZhxuFNPLb8O4Xw33Ldro3u4nStkA902W+AxXJBVxWe2jQ6Ax
RJOX1ff7oNxvLYx6/9zhw2S5DlbB9TBCCw4tZ6SMmp2lI2dvX7e+plStK0YmyR4G
REgZnEICuww81xJpuhpv9fNcaq76pR2aNf3FXSt56qqz5JogZl1yUmA5uyGjbz7l
f4FIxpKrpT2au80ZgsbFqOE9Xm4EKGWv9S2KCRQV7ZiLzQSkEy3h5hz8d17Ylo5p
0lehKNmJGhmKi62V3awF1weJ1PVqOBHL7p9RLf/TSiVDPpLb8TghoyVqP1X12TtD
WfMk2Ah2ACTs/c+14MwPdFjFVBIuY1XQVF6wNmgTS5RziqIgS8SPH0u9snB4Comw
011lLZ5WzskzcqoFnmunaflL3sIVCydkTLwSAf+qPky/MCFpjmM=
=Wq0I
-----END PGP SIGNATURE-----

--UW9e6asxQmvxXgYU--