| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> | 
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de> | 
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> | 
| Subject: | Re: Frontend error logging style | 
| Date: | 2022-02-23 15:30:02 | 
| Message-ID: | 1003080.1645630202@sss.pgh.pa.us | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
I wrote:
> Andres Freund <andres(at)anarazel(dot)de> 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrew Dunstan | 2022-02-23 15:54:15 | Re: making pg_regress less noisy by removing boilerplate | 
| Previous Message | Gunnar "Nick" Bluth | 2022-02-23 15:28:09 | PATCH: add "--config-file=" option to pg_rewind |