public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Frontend error logging style
Date: Sat, 26 Feb 2022 14:21:01 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CA+TgmoYBmULn65p4cz3QnL50vJozO=DmO-mKSt9HbVWBMHbgXw@mail.gmail.com>
	<YZMfMI3kTDw/[email protected]>
	<CA+TgmoZy84orCfUNipEp+QzWwRuR2yrjsdcJN9ROHzPkXDRbxQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoYaA1wtU91yNg1_C6oS8azAfp5dQ1YrfuNAm2=Yefrn1A@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (5+ 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], [email protected]
  Subject: Re: Frontend error logging style
  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