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

5 files changed, 28 insertions, 42 deletions

diff --git a/src/bin/pg_rewind/nls.mk b/src/bin/pg_rewind/nls.mk
index a561f965df..a50f9139df 100644
--- a/src/bin/pg_rewind/nls.mk
+++ b/src/bin/pg_rewind/nls.mk
@@ -2,7 +2,6 @@
 CATALOG_NAME     = pg_rewind
 AVAIL_LANGUAGES  = cs de es fr it ja ko pl pt_BR ru sv tr uk zh_CN
 GETTEXT_FILES    = $(FRONTEND_COMMON_GETTEXT_FILES) datapagemap.c file_ops.c filemap.c libpq_source.c local_source.c parsexlog.c pg_rewind.c timeline.c xlogreader.c ../../common/fe_memutils.c ../../common/restricted_token.c ../../fe_utils/archive.c ../../fe_utils/recovery_gen.c
-GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS) pg_fatal report_invalid_record:2
+GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS) report_invalid_record:2
 GETTEXT_FLAGS    = $(FRONTEND_COMMON_GETTEXT_FLAGS) \
-    pg_fatal:1:c-format \
     report_invalid_record:2:c-format
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 0922032e23..2ca4dd29af 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -165,10 +165,6 @@ main(int argc, char **argv)
 	{
 		switch (c)
 		{
-			case '?':
-				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
-				exit(1);
-
 			case 'c':
 				restore_wal = true;
 				break;
@@ -213,34 +209,39 @@ main(int argc, char **argv)
 			case 5:
 				config_file = pg_strdup(optarg);
 				break;
+
+			default:
+				/* getopt_long already emitted a complaint */
+				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+				exit(1);
 		}
 	}
 
 	if (datadir_source == NULL && connstr_source == NULL)
 	{
 		pg_log_error("no source specified (--source-pgdata or --source-server)");
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
 	if (datadir_source != NULL && connstr_source != NULL)
 	{
 		pg_log_error("only one of --source-pgdata or --source-server can be specified");
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
 	if (datadir_target == NULL)
 	{
 		pg_log_error("no target data directory specified (--target-pgdata)");
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
 	if (writerecoveryconf && connstr_source == NULL)
 	{
 		pg_log_error("no source server information (--source-server) specified for --write-recovery-conf");
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
@@ -248,7 +249,7 @@ main(int argc, char **argv)
 	{
 		pg_log_error("too many command-line arguments (first is \"%s\")",
 					 argv[optind]);
-		fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
+		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
 		exit(1);
 	}
 
@@ -262,8 +263,8 @@ main(int argc, char **argv)
 	if (geteuid() == 0)
 	{
 		pg_log_error("cannot be executed by \"root\"");
-		fprintf(stderr, _("You must run %s as the PostgreSQL superuser.\n"),
-				progname);
+		pg_log_error_hint("You must run %s as the PostgreSQL superuser.",
+						  progname);
 		exit(1);
 	}
 #endif
@@ -272,11 +273,8 @@ main(int argc, char **argv)
 
 	/* Set mask based on PGDATA permissions */
 	if (!GetDataDirectoryCreatePerm(datadir_target))
-	{
-		pg_log_error("could not read permissions of directory \"%s\": %m",
-					 datadir_target);
-		exit(1);
-	}
+		pg_fatal("could not read permissions of directory \"%s\": %m",
+				 datadir_target);
 
 	umask(pg_mode_mask);
 
@@ -1041,16 +1039,11 @@ getRestoreCommand(const char *argv0)
 			strlcpy(full_path, progname, sizeof(full_path));
 
 		if (rc == -1)
-			pg_log_error("The program \"%s\" is needed by %s but was not found in the\n"
-						 "same directory as \"%s\".\n"
-						 "Check your installation.",
-						 "postgres", progname, full_path);
+			pg_fatal("program \"%s\" is needed by %s but was not found in the same directory as \"%s\"",
+					 "postgres", progname, full_path);
 		else
-			pg_log_error("The program \"%s\" was found by \"%s\"\n"
-						 "but was not the same version as %s.\n"
-						 "Check your installation.",
-						 "postgres", full_path, progname);
-		exit(1);
+			pg_fatal("program \"%s\" was found by \"%s\" but was not the same version as %s",
+					 "postgres", full_path, progname);
 	}
 
 	/*
@@ -1116,14 +1109,10 @@ ensureCleanShutdown(const char *argv0)
 			strlcpy(full_path, progname, sizeof(full_path));
 
 		if (ret == -1)
-			pg_fatal("The program \"%s\" is needed by %s but was not found in the\n"
-					 "same directory as \"%s\".\n"
-					 "Check your installation.",
+			pg_fatal("program \"%s\" is needed by %s but was not found in the same directory as \"%s\"",
 					 "postgres", progname, full_path);
 		else
-			pg_fatal("The program \"%s\" was found by \"%s\"\n"
-					 "but was not the same version as %s.\n"
-					 "Check your installation.",
+			pg_fatal("program \"%s\" was found by \"%s\" but was not the same version as %s",
 					 "postgres", full_path, progname);
 	}
 
@@ -1165,7 +1154,8 @@ ensureCleanShutdown(const char *argv0)
 	if (system(postgres_cmd->data) != 0)
 	{
 		pg_log_error("postgres single-user mode in target cluster failed");
-		pg_fatal("Command was: %s", postgres_cmd->data);
+		pg_log_error_detail("Command was: %s", postgres_cmd->data);
+		exit(1);
 	}
 
 	destroyPQExpBuffer(postgres_cmd);
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index 388870ce95..393182fe2a 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -33,9 +33,6 @@ extern int	targetNentries;
 extern uint64 fetch_size;
 extern uint64 fetch_done;
 
-/* logging support */
-#define pg_fatal(...) do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
-
 /* in parsexlog.c */
 extern void extractPageMap(const char *datadir, XLogRecPtr startpoint,
 						   int tliIndex, XLogRecPtr endpoint,
diff --git a/src/bin/pg_rewind/t/009_growing_files.pl b/src/bin/pg_rewind/t/009_growing_files.pl
index 752781e637..2c81406cc0 100644
--- a/src/bin/pg_rewind/t/009_growing_files.pl
+++ b/src/bin/pg_rewind/t/009_growing_files.pl
@@ -71,6 +71,6 @@ my $last;
 open my $f, '<', "$standby_pgdata/tst_both_dir/file1";
 $last = $_ while (<$f>);
 close $f;
-like($last, qr/fatal: size of source file/, "Check error message");
+like($last, qr/error: size of source file/, "Check error message");
 
 done_testing();
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index df8f82a50c..983388c92b 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -73,19 +73,19 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 		{
 			/* expect a numeric timeline ID as first field of line */
 			pg_log_error("syntax error in history file: %s", fline);
-			pg_log_error("Expected a numeric timeline ID.");
+			pg_log_error_detail("Expected a numeric timeline ID.");
 			exit(1);
 		}
 		if (nfields != 3)
 		{
 			pg_log_error("syntax error in history file: %s", fline);
-			pg_log_error("Expected a write-ahead log switchpoint location.");
+			pg_log_error_detail("Expected a write-ahead log switchpoint location.");
 			exit(1);
 		}
 		if (entries && tli <= lasttli)
 		{
 			pg_log_error("invalid data in history file: %s", fline);
-			pg_log_error("Timeline IDs must be in increasing sequence.");
+			pg_log_error_detail("Timeline IDs must be in increasing sequence.");
 			exit(1);
 		}
 
@@ -106,7 +106,7 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
 	if (entries && targetTLI <= lasttli)
 	{
 		pg_log_error("invalid data in history file");
-		pg_log_error("Timeline IDs must be less than child timeline's ID.");
+		pg_log_error_detail("Timeline IDs must be less than child timeline's ID.");
 		exit(1);
 	}