public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tom Lane <[email protected]>
To: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Frontend error logging style
Date: Wed, 23 Feb 2022 10:30:02 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CA+TgmoZjmJF0bTfd_q7yXWCR9cp0n6XgBHM7XPPy7R9jSWrJCQ@mail.gmail.com>
	<[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]>

I wrote:
> Andres Freund <[email protected]> writes:
>> What about adding a pg_fatal() that's pg_log_fatal() + exit()? That keeps
>> pg_log_* stuff "log only", but adds something adjacent enough to hopefully
>> reduce future misunderstandings?

> I'd be okay with that, except that pg_upgrade already has a pg_fatal
> (because it has its *own* logging system, just in case you thought
> this wasn't enough of a mess yet).

Wait a moment.  After looking closer, I realize that pg_upgrade's
pg_fatal could trivially be turned into a macro; and the other two
existing definitions already are macros.  That would remove the risk
of link-time symbol collisions that I was worried about.  As a bonus,
it'd substantially reduce the number of changes needed to make
pg_upgrade use logging.c, whenever somebody wants to make that happen.

So I now propose modifying yesterday's patch thus:

* Reinstantiate the PG_LOG_FATAL enum value, add support macros
pg_log_fatal, pg_log_fatal_hint, pg_log_fatal_detail.

* Define pg_fatal as pg_log_fatal + exit(1).  (This would essentially
move pg_rewind's definition into logging.h.  pg_upgrade will
define it slightly differently, but the semantics end up the same.)

* Adjust call sites to match.

I do like this idea because it would not break any existing code
that expects pg_log_fatal to return.  There is likely to be some
of that in outstanding patches, and this approach would merely
render it less-than-idiomatic rather than outright broken.

Updating the patch is going to be a bit tedious, so I'm not
going to do it without buy-in that this solution would be
okay to commit.  Any objections?

			regards, tom lane






view thread (7+ messages)  latest in thread

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], [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