Hi, On 2022-02-22 18:23:19 -0500, Tom Lane wrote: > Robert Haas <robertmh...@gmail.com> writes: > > On Fri, Nov 19, 2021 at 5:17 AM Peter Eisentraut > > <peter.eisentr...@enterprisedb.com> wrote: > >> If people want to do that kind of thing (I'm undecided whether the > >> complexity is worth it), then make it a different API. The pg_log_* > >> calls are for writing formatted output. They normalized existing > >> hand-coded patterns at the time. We can wrap another API on top of them > >> that does flow control and output. The pg_log_* stuff is more on the > >> level of syslog(), which also just outputs stuff. Nobody is suggesting > >> that syslog(LOG_EMERG) should exit the program automatically. But you > >> can wrap higher-level APIs such as ereport() on top of that that might > >> do that.
That'd maybe be a convincing argument in a project that didn't have elog(FATAL). > > Yeah, that might be a way forward. > > This conversation seems to have tailed off without full resolution, > but I observe that pretty much everyone except Peter is on board > with defining pg_log_fatal as pg_log_error + exit(1). So I think > we should just do that, unless Peter wants to produce a finished > alternative proposal. 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? To further decrease the chance of such mistakes, we could rename pg_log_fatal() to something else. Or add a a pg_log_fatal() function that's marked pg_nodiscard(), so that each callsite explicitly has to be (void) pg_log_fatal(). > I've now gone through and fleshed out the patch I sketched upthread. > This exercise convinced me that we absolutely should do something > very like this, because > > (1) As it stands, this patch removes a net of just about 1000 lines > of code. So we clearly need *something*. > (2) The savings would be even more, except that people have invented > macros for pg_log_error + exit(1) in at least four places already. > (None of them quite the same of course.) Here I've just fixed those > macro definitions to use pg_log_fatal. In the name of consistency, we > should probably get rid of those macros in favor of using pg_log_fatal > directly; but I've not done so here, since this patch is eyewateringly > long and boring already. Agreed. I don't care that much about the naming here. For a moment I was wondering whether there'd be a benefit for having the "pure logging" stuff be in a different static library than the erroring variant. So that e.g. libpq's check for exit() doesn't run into trouble. But then I remembered that libpq doesn't link against common... Greetings, Andres Freund
