Improve frontend error logging style.

Get rid of the separate "FATAL" log level, as it was applied
so inconsistently as to be meaningless.  This mostly involves
s/pg_log_fatal/pg_log_error/g.

Create a macro pg_fatal() to handle the common use-case of
pg_log_error() immediately followed by exit(1).  Various
modules had already invented either this or equivalent macros;
standardize on pg_fatal() and apply it where possible.

Invent the ability to add "detail" and "hint" messages to a
frontend message, much as we have long had in the backend.

Except where rewording was needed to convert existing coding
to detail/hint style, I have (mostly) resisted the temptation
to change existing message wording.

Patch by me.  Design and patch reviewed at various stages by
Robert Haas, Kyotaro Horiguchi, Peter Eisentraut and
Daniel Gustafsson.

Discussion: https://postgr.es/m/1363732.1636496441@sss.pgh.pa.us

4 files changed, 49 insertions, 62 deletions

diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 348f046a440..4c0da6e124e 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -70,11 +70,8 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
 						ControlFilePath)));
 #else
 	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
-	{
-		pg_log_fatal("could not open file \"%s\" for reading: %m",
-					 ControlFilePath);
-		exit(EXIT_FAILURE);
-	}
+		pg_fatal("could not open file \"%s\" for reading: %m",
+				 ControlFilePath);
 #endif
 
 	r = read(fd, ControlFile, sizeof(ControlFileData));
@@ -86,10 +83,7 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
 					(errcode_for_file_access(),
 					 errmsg("could not read file \"%s\": %m", ControlFilePath)));
 #else
-		{
-			pg_log_fatal("could not read file \"%s\": %m", ControlFilePath);
-			exit(EXIT_FAILURE);
-		}
+			pg_fatal("could not read file \"%s\": %m", ControlFilePath);
 #endif
 		else
 #ifndef FRONTEND
@@ -98,11 +92,8 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
 					 errmsg("could not read file \"%s\": read %d of %zu",
 							ControlFilePath, r, sizeof(ControlFileData))));
 #else
-		{
-			pg_log_fatal("could not read file \"%s\": read %d of %zu",
-						 ControlFilePath, r, sizeof(ControlFileData));
-			exit(EXIT_FAILURE);
-		}
+			pg_fatal("could not read file \"%s\": read %d of %zu",
+					 ControlFilePath, r, sizeof(ControlFileData));
 #endif
 	}
 
@@ -114,10 +105,7 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
 						ControlFilePath)));
 #else
 	if (close(fd) != 0)
-	{
-		pg_log_fatal("could not close file \"%s\": %m", ControlFilePath);
-		exit(EXIT_FAILURE);
-	}
+		pg_fatal("could not close file \"%s\": %m", ControlFilePath);
 #endif
 
 	/* Check the CRC. */
@@ -203,10 +191,7 @@ update_controlfile(const char *DataDir,
 #else
 	if ((fd = open(ControlFilePath, O_WRONLY | PG_BINARY,
 				   pg_file_create_mode)) == -1)
-	{
-		pg_log_fatal("could not open file \"%s\": %m", ControlFilePath);
-		exit(EXIT_FAILURE);
-	}
+		pg_fatal("could not open file \"%s\": %m", ControlFilePath);
 #endif
 
 	errno = 0;
@@ -225,8 +210,7 @@ update_controlfile(const char *DataDir,
 				 errmsg("could not write file \"%s\": %m",
 						ControlFilePath)));
 #else
-		pg_log_fatal("could not write file \"%s\": %m", ControlFilePath);
-		exit(EXIT_FAILURE);
+		pg_fatal("could not write file \"%s\": %m", ControlFilePath);
 #endif
 	}
 #ifndef FRONTEND
@@ -245,10 +229,7 @@ update_controlfile(const char *DataDir,
 		pgstat_report_wait_end();
 #else
 		if (fsync(fd) != 0)
-		{
-			pg_log_fatal("could not fsync file \"%s\": %m", ControlFilePath);
-			exit(EXIT_FAILURE);
-		}
+			pg_fatal("could not fsync file \"%s\": %m", ControlFilePath);
 #endif
 	}
 
@@ -260,8 +241,7 @@ update_controlfile(const char *DataDir,
 				 errmsg("could not close file \"%s\": %m",
 						ControlFilePath)));
 #else
-		pg_log_fatal("could not close file \"%s\": %m", ControlFilePath);
-		exit(EXIT_FAILURE);
+		pg_fatal("could not close file \"%s\": %m", ControlFilePath);
 #endif
 	}
 }
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 7138068633b..19d308ad1f9 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -300,7 +300,7 @@ fsync_fname(const char *fname, bool isdir)
 	 */
 	if (returncode != 0 && !(isdir && (errno == EBADF || errno == EINVAL)))
 	{
-		pg_log_fatal("could not fsync file \"%s\": %m", fname);
+		pg_log_error("could not fsync file \"%s\": %m", fname);
 		(void) close(fd);
 		exit(EXIT_FAILURE);
 	}
@@ -370,7 +370,7 @@ durable_rename(const char *oldfile, const char *newfile)
 	{
 		if (fsync(fd) != 0)
 		{
-			pg_log_fatal("could not fsync file \"%s\": %m", newfile);
+			pg_log_error("could not fsync file \"%s\": %m", newfile);
 			close(fd);
 			exit(EXIT_FAILURE);
 		}
@@ -448,7 +448,7 @@ get_dirent_type(const char *path,
 		{
 			result = PGFILETYPE_ERROR;
 #ifdef FRONTEND
-			pg_log_generic(elevel, "could not stat file \"%s\": %m", path);
+			pg_log_generic(elevel, PG_LOG_PRIMARY, "could not stat file \"%s\": %m", path);
 #else
 			ereport(elevel,
 					(errcode_for_file_access(),
diff --git a/src/common/logging.c b/src/common/logging.c
index 9a076bb8128..18d6669f276 100644
--- a/src/common/logging.c
+++ b/src/common/logging.c
@@ -151,6 +151,9 @@ pg_logging_init(const char *argv0)
 	}
 }
 
+/*
+ * Change the logging flags.
+ */
 void
 pg_logging_config(int new_flags)
 {
@@ -194,17 +197,19 @@ pg_logging_set_locus_callback(void (*cb) (const char **filename, uint64 *lineno)
 }
 
 void
-pg_log_generic(enum pg_log_level level, const char *pg_restrict fmt,...)
+pg_log_generic(enum pg_log_level level, enum pg_log_part part,
+			   const char *pg_restrict fmt,...)
 {
 	va_list		ap;
 
 	va_start(ap, fmt);
-	pg_log_generic_v(level, fmt, ap);
+	pg_log_generic_v(level, part, fmt, ap);
 	va_end(ap);
 }
 
 void
-pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_list ap)
+pg_log_generic_v(enum pg_log_level level, enum pg_log_part part,
+				 const char *pg_restrict fmt, va_list ap)
 {
 	int			save_errno = errno;
 	const char *filename = NULL;
@@ -232,7 +237,8 @@ pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_list a
 
 	fmt = _(fmt);
 
-	if (!(log_flags & PG_LOG_FLAG_TERSE) || filename)
+	if (part == PG_LOG_PRIMARY &&
+		(!(log_flags & PG_LOG_FLAG_TERSE) || filename))
 	{
 		if (sgr_locus)
 			fprintf(stderr, ANSI_ESCAPE_FMT, sgr_locus);
@@ -251,30 +257,34 @@ pg_log_generic_v(enum pg_log_level level, const char *pg_restrict fmt, va_list a
 
 	if (!(log_flags & PG_LOG_FLAG_TERSE))
 	{
-		switch (level)
+		switch (part)
 		{
-			case PG_LOG_FATAL:
-				if (sgr_error)
-					fprintf(stderr, ANSI_ESCAPE_FMT, sgr_error);
-				fprintf(stderr, _("fatal: "));
-				if (sgr_error)
-					fprintf(stderr, ANSI_ESCAPE_RESET);
-				break;
-			case PG_LOG_ERROR:
-				if (sgr_error)
-					fprintf(stderr, ANSI_ESCAPE_FMT, sgr_error);
-				fprintf(stderr, _("error: "));
-				if (sgr_error)
-					fprintf(stderr, ANSI_ESCAPE_RESET);
+			case PG_LOG_PRIMARY:
+				switch (level)
+				{
+					case PG_LOG_ERROR:
+						if (sgr_error)
+							fprintf(stderr, ANSI_ESCAPE_FMT, sgr_error);
+						fprintf(stderr, _("error: "));
+						if (sgr_error)
+							fprintf(stderr, ANSI_ESCAPE_RESET);
+						break;
+					case PG_LOG_WARNING:
+						if (sgr_warning)
+							fprintf(stderr, ANSI_ESCAPE_FMT, sgr_warning);
+						fprintf(stderr, _("warning: "));
+						if (sgr_warning)
+							fprintf(stderr, ANSI_ESCAPE_RESET);
+						break;
+					default:
+						break;
+				}
 				break;
-			case PG_LOG_WARNING:
-				if (sgr_warning)
-					fprintf(stderr, ANSI_ESCAPE_FMT, sgr_warning);
-				fprintf(stderr, _("warning: "));
-				if (sgr_warning)
-					fprintf(stderr, ANSI_ESCAPE_RESET);
+			case PG_LOG_DETAIL:
+				fprintf(stderr, _("detail: "));
 				break;
-			default:
+			case PG_LOG_HINT:
+				fprintf(stderr, _("hint: "));
 				break;
 		}
 	}
diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
index 48b1ce0585f..82b74b565eb 100644
--- a/src/common/restricted_token.c
+++ b/src/common/restricted_token.c
@@ -190,10 +190,7 @@ get_restricted_token(void)
 			WaitForSingleObject(pi.hProcess, INFINITE);
 
 			if (!GetExitCodeProcess(pi.hProcess, &x))
-			{
-				pg_log_error("could not get exit code from subprocess: error code %lu", GetLastError());
-				exit(1);
-			}
+				pg_fatal("could not get exit code from subprocess: error code %lu", GetLastError());
 			exit(x);
 		}
 		pg_free(cmdline);