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

1 files changed, 24 insertions, 22 deletions

diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 3184eda3e75..89cdbf80e0e 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -39,7 +39,7 @@ _check_database_version(ArchiveHandle *AH)
 	remoteversion_str = PQparameterStatus(AH->connection, "server_version");
 	remoteversion = PQserverVersion(AH->connection);
 	if (remoteversion == 0 || !remoteversion_str)
-		fatal("could not get server_version from libpq");
+		pg_fatal("could not get server_version from libpq");
 
 	AH->public.remoteVersionStr = pg_strdup(remoteversion_str);
 	AH->public.remoteVersion = remoteversion;
@@ -50,9 +50,10 @@ _check_database_version(ArchiveHandle *AH)
 		&& (remoteversion < AH->public.minRemoteVersion ||
 			remoteversion > AH->public.maxRemoteVersion))
 	{
-		pg_log_error("server version: %s; %s version: %s",
-					 remoteversion_str, progname, PG_VERSION);
-		fatal("aborting because of server version mismatch");
+		pg_log_error("aborting because of server version mismatch");
+		pg_log_error_detail("server version: %s; %s version: %s",
+							remoteversion_str, progname, PG_VERSION);
+		exit(1);
 	}
 
 	/*
@@ -116,7 +117,7 @@ ConnectDatabase(Archive *AHX,
 	bool		new_pass;
 
 	if (AH->connection)
-		fatal("already connected to a database");
+		pg_fatal("already connected to a database");
 
 	/* Never prompt for a password during a reconnection */
 	prompt_password = isReconnect ? TRI_NO : cparams->promptPassword;
@@ -166,7 +167,7 @@ ConnectDatabase(Archive *AHX,
 		AH->connection = PQconnectdbParams(keywords, values, true);
 
 		if (!AH->connection)
-			fatal("could not connect to database");
+			pg_fatal("could not connect to database");
 
 		if (PQstatus(AH->connection) == CONNECTION_BAD &&
 			PQconnectionNeedsPassword(AH->connection) &&
@@ -183,11 +184,11 @@ ConnectDatabase(Archive *AHX,
 	if (PQstatus(AH->connection) == CONNECTION_BAD)
 	{
 		if (isReconnect)
-			fatal("reconnection failed: %s",
-				  PQerrorMessage(AH->connection));
+			pg_fatal("reconnection failed: %s",
+					 PQerrorMessage(AH->connection));
 		else
-			fatal("%s",
-				  PQerrorMessage(AH->connection));
+			pg_fatal("%s",
+					 PQerrorMessage(AH->connection));
 	}
 
 	/* Start strict; later phases may override this. */
@@ -235,7 +236,7 @@ DisconnectDatabase(Archive *AHX)
 		/*
 		 * If we have an active query, send a cancel before closing, ignoring
 		 * any errors.  This is of no use for a normal exit, but might be
-		 * helpful during fatal().
+		 * helpful during pg_fatal().
 		 */
 		if (PQtransactionStatus(AH->connection) == PQTRANS_ACTIVE)
 			(void) PQcancel(AH->connCancel, errbuf, sizeof(errbuf));
@@ -261,16 +262,17 @@ GetConnection(Archive *AHX)
 static void
 notice_processor(void *arg, const char *message)
 {
-	pg_log_generic(PG_LOG_INFO, "%s", message);
+	pg_log_info("%s", message);
 }
 
-/* Like fatal(), but with a complaint about a particular query. */
+/* Like pg_fatal(), but with a complaint about a particular query. */
 static void
 die_on_query_failure(ArchiveHandle *AH, const char *query)
 {
 	pg_log_error("query failed: %s",
 				 PQerrorMessage(AH->connection));
-	fatal("query was: %s", query);
+	pg_log_error_detail("Query was: %s", query);
+	exit(1);
 }
 
 void
@@ -311,10 +313,10 @@ ExecuteSqlQueryForSingleRow(Archive *fout, const char *query)
 	/* Expecting a single result only */
 	ntups = PQntuples(res);
 	if (ntups != 1)
-		fatal(ngettext("query returned %d row instead of one: %s",
-					   "query returned %d rows instead of one: %s",
-					   ntups),
-			  ntups, query);
+		pg_fatal(ngettext("query returned %d row instead of one: %s",
+						  "query returned %d rows instead of one: %s",
+						  ntups),
+				 ntups, query);
 
 	return res;
 }
@@ -456,8 +458,8 @@ ExecuteSqlCommandBuf(Archive *AHX, const char *buf, size_t bufLen)
 		 */
 		if (AH->pgCopyIn &&
 			PQputCopyData(AH->connection, buf, bufLen) <= 0)
-			fatal("error returned by PQputCopyData: %s",
-				  PQerrorMessage(AH->connection));
+			pg_fatal("error returned by PQputCopyData: %s",
+					 PQerrorMessage(AH->connection));
 	}
 	else if (AH->outputKind == OUTPUT_OTHERDATA)
 	{
@@ -505,8 +507,8 @@ EndDBCopyMode(Archive *AHX, const char *tocEntryTag)
 		PGresult   *res;
 
 		if (PQputCopyEnd(AH->connection, NULL) <= 0)
-			fatal("error returned by PQputCopyEnd: %s",
-				  PQerrorMessage(AH->connection));
+			pg_fatal("error returned by PQputCopyEnd: %s",
+					 PQerrorMessage(AH->connection));
 
 		/* Check command status and return to normal libpq state */
 		res = PQgetResult(AH->connection);