From 501704e826fc75d7d1c8976a8d177f8ce3ea0b4c Mon Sep 17 00:00:00 2001
From: Jeff Davis <jdavis@postgresql.org>
Date: Tue, 22 Aug 2023 11:21:36 -0700
Subject: [PATCH] Fix pg_dump assertion failure when dumping pg_catalog.

Commit 396d348b04 did not account for the default collation.

Also, use pg_log_warning() instead of Assert().

Discussion: https://postgr.es/m/ce071503fee88334aa70f360e6e4ea14d48305ee.camel%40j-davis.com
Reviewed-by: Michael Paquier
Backpatch-through: 15
---
 src/bin/pg_dump/pg_dump.c        | 73 ++++++++++++++++++++++++--------
 src/bin/pg_dump/t/002_pg_dump.pl |  8 ++++
 2 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5dab1ba9ea..c73e9a11da 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -13415,6 +13415,18 @@ dumpCollation(Archive *fout, const CollInfo *collinfo)
 	else
 		collctype = NULL;
 
+	/*
+	 * Before version 15, collcollate and collctype were of type NAME and
+	 * non-nullable. Treat empty strings as NULL for consistency.
+	 */
+	if (fout->remoteVersion < 150000)
+	{
+		if (collcollate[0] == '\0')
+			collcollate = NULL;
+		if (collctype[0] == '\0')
+			collctype = NULL;
+	}
+
 	if (!PQgetisnull(res, 0, i_colliculocale))
 		colliculocale = PQgetvalue(res, 0, i_colliculocale);
 	else
@@ -13446,35 +13458,60 @@ dumpCollation(Archive *fout, const CollInfo *collinfo)
 	if (strcmp(PQgetvalue(res, 0, i_collisdeterministic), "f") == 0)
 		appendPQExpBufferStr(q, ", deterministic = false");
 
-	if (colliculocale != NULL)
+	if (collprovider[0] == 'd')
 	{
-		appendPQExpBufferStr(q, ", locale = ");
-		appendStringLiteralAH(q, colliculocale, fout);
-	}
-	else
-	{
-		Assert(collcollate != NULL);
-		Assert(collctype != NULL);
+		if (collcollate || collctype || colliculocale || collicurules)
+			pg_log_warning("invalid collation \"%s\"", qcollname);
 
-		if (strcmp(collcollate, collctype) == 0)
+		/* no locale -- the default collation cannot be reloaded anyway */
+	}
+	else if (collprovider[0] == 'i')
+	{
+		if (fout->remoteVersion >= 150000)
+		{
+			if (collcollate || collctype || !colliculocale)
+				pg_log_warning("invalid collation \"%s\"", qcollname);
+
+			appendPQExpBufferStr(q, ", locale = ");
+			appendStringLiteralAH(q, colliculocale ? colliculocale : "",
+								  fout);
+		}
+		else
+		{
+			if (!collcollate || !collctype || colliculocale ||
+				strcmp(collcollate, collctype) != 0)
+				pg_log_warning("invalid collation \"%s\"", qcollname);
+
+			appendPQExpBufferStr(q, ", locale = ");
+			appendStringLiteralAH(q, collcollate ? collcollate : "", fout);
+		}
+
+		if (collicurules)
+		{
+			appendPQExpBufferStr(q, ", rules = ");
+			appendStringLiteralAH(q, collicurules ? collicurules : "", fout);
+		}
+	}
+	else if (collprovider[0] == 'c')
+	{
+		if (colliculocale || collicurules || !collcollate || !collctype)
+			pg_log_warning("invalid collation \"%s\"", qcollname);
+
+		if (collcollate && collctype && strcmp(collcollate, collctype) == 0)
 		{
 			appendPQExpBufferStr(q, ", locale = ");
-			appendStringLiteralAH(q, collcollate, fout);
+			appendStringLiteralAH(q, collcollate ? collcollate : "", fout);
 		}
 		else
 		{
 			appendPQExpBufferStr(q, ", lc_collate = ");
-			appendStringLiteralAH(q, collcollate, fout);
+			appendStringLiteralAH(q, collcollate ? collcollate : "", fout);
 			appendPQExpBufferStr(q, ", lc_ctype = ");
-			appendStringLiteralAH(q, collctype, fout);
+			appendStringLiteralAH(q, collctype ? collctype : "", fout);
 		}
 	}
-
-	if (collicurules)
-	{
-		appendPQExpBufferStr(q, ", rules = ");
-		appendStringLiteralAH(q, collicurules, fout);
-	}
+	else
+		pg_fatal("unrecognized collation provider '%c'", collprovider[0]);
 
 	/*
 	 * For binary upgrade, carry over the collation version.  For normal
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 0efeb3367d..c74b7234f6 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4753,6 +4753,14 @@ $node->command_fails_like(
 	qr/pg_dumpall: error: improper qualified name \(too many dotted names\): myhost\.mydb/,
 	'pg_dumpall: option --exclude-database rejects multipart database names');
 
+##############################################################
+# Test dumping pg_catalog (for research -- cannot be reloaded)
+
+$node->command_ok(
+	[ 'pg_dump', '-p', "$port", '-n', 'pg_catalog' ],
+	'pg_dump: option -n pg_catalog'
+);
+
 #########################################
 # Test valid database exclusion patterns