ISTM that the recently-introduced new frontend logging support
(common/logging.h et al) could use a second pass now that we have
some experience with it. There are two points that are bugging me:
1. The distinction between "error" and "fatal" levels seems squishy
to the point of uselessness. I think we should either get rid of it
entirely, or make an effort to use "fatal" exactly for the cases that
are going to give up and exit right away. Of the approximately 830
pg_log_error calls in HEAD, I count at least 450 that are immediately
followed by exit(1), and so should be pg_log_fatal if this distinction
means anything at all. OTOH, if we decide it doesn't mean anything,
there are only about 90 pg_log_fatal calls to convert. I lean
slightly to the "get rid of the distinction" option, not only because
it'd be a much smaller patch but also because I don't care to expend
brain cells on the which-to-use question while reviewing future
patches.
2. What is the preferred style for adding extra lines to log messages?
I see a lot of direct prints to stderr:
pg_log_error("missing required argument: database name");
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
progname);
exit(1);
but a number of places have chosen to do this:
pg_log_error("query failed: %s", PQerrorMessage(conn));
pg_log_error("query was: %s", todo);
and some places got creative and did this:
pg_log_error("query failed: %s", PQerrorMessage(conn));
pg_log_info("query was: %s", sql.data);
I think this ought to be cleaned up so we have a more-or-less uniform
approach. Aside from the randomly different source code, each of
these choices has different implications for whether the extra line
gets printed or not depending on verbosity level, and that seems bad.
One plausible choice is to drop the first style (which surely has
little to recommend it) and use either the second or third style
depending on whether you think the addendum ought to appear at the
same or higher verbosity level as the main message. But we'd
still be at hazard of people making randomly different choices
about that in identical cases, as indeed the second and third
examples show.
Another idea is to reduce such cases to one call:
pg_log_error("query failed: %s\nquery was: %s",
PQerrorMessage(conn), sql.data);
but I don't much like that: it knows more than it should about
the presentation format, and it can't support hiding the detail
portion at lower verbosity levels.
So I'm not totally satisfied with these ideas, but I don't
immediately have a better one.
Thoughts?
regards, tom lane
