Dump COMMENT ON SCHEMA public.

As we do for other attributes of the public schema, omit the COMMENT
command when its payload would match what initdb had installed.  For
dumps that do carry this new COMMENT command, non-superusers restoring
them are likely to get an error.

Reviewed by Asif Rehman.

Discussion: https://postgr.es/m/ab48a34c-60f6-e388-502a-3e5fe46a2dae@postgresfriends.org

src/bin/pg_dump/pg_dump.c

@@ -169,9 +169,15 @@ static NamespaceInfo *findNamespace(Oid nsoid);
 static void dumpTableData(Archive *fout, const TableDataInfo *tdinfo);
 static void refreshMatViewData(Archive *fout, const TableDataInfo *tdinfo);
 static void guessConstraintInheritance(TableInfo *tblinfo, int numTables);
-static void dumpComment(Archive *fout, const char *type, const char *name,
+static void dumpCommentExtended(Archive *fout, const char *type,
-						const char *namespace, const char *owner,
+								const char *name, const char *namespace,
-						CatalogId catalogId, int subid, DumpId dumpId);
+								const char *owner, CatalogId catalogId,
+								int subid, DumpId dumpId,
+								const char *initdb_comment);
+static inline void dumpComment(Archive *fout, const char *type,
+							   const char *name, const char *namespace,
+							   const char *owner, CatalogId catalogId,
+							   int subid, DumpId dumpId);
 static int	findComments(Archive *fout, Oid classoid, Oid objoid,
 						 CommentItem **items);
 static int	collectComments(Archive *fout, CommentItem **items);
@@ -1638,16 +1644,13 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout)
 	{
 		/*
 		 * The public schema is a strange beast that sits in a sort of
-		 * no-mans-land between being a system object and a user object.  We
+		 * no-mans-land between being a system object and a user object.
-		 * don't want to dump creation or comment commands for it, because
+		 * CREATE SCHEMA would fail, so its DUMP_COMPONENT_DEFINITION is just
-		 * that complicates matters for non-superuser use of pg_dump.  But we
+		 * a comment and an indication of ownership.  If the owner is the
-		 * should dump any ownership changes, security labels, and ACL
+		 * default, that DUMP_COMPONENT_DEFINITION is superfluous.
-		 * changes, and of course we should dump contained objects.  pg_dump
-		 * ties ownership to DUMP_COMPONENT_DEFINITION.  Hence, unless the
-		 * owner is the default, dump a "definition" bearing just a comment.
 		 */
 		nsinfo->create = false;
-		nsinfo->dobj.dump = DUMP_COMPONENT_ALL & ~DUMP_COMPONENT_COMMENT;
+		nsinfo->dobj.dump = DUMP_COMPONENT_ALL;
 		if (nsinfo->nspowner == BOOTSTRAP_SUPERUSERID)
 			nsinfo->dobj.dump &= ~DUMP_COMPONENT_DEFINITION;
 		nsinfo->dobj.dump_contains = DUMP_COMPONENT_ALL;
@@ -9873,7 +9876,7 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
 }
 
 /*
- * dumpComment --
+ * dumpCommentExtended --
  *
  * This routine is used to dump any comments associated with the
  * object handed to this routine. The routine takes the object type
@@ -9896,9 +9899,11 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
  * calling ArchiveEntry() for the specified object.
  */
 static void
-dumpComment(Archive *fout, const char *type, const char *name,
+dumpCommentExtended(Archive *fout, const char *type,
-			const char *namespace, const char *owner,
+					const char *name, const char *namespace,
-			CatalogId catalogId, int subid, DumpId dumpId)
+					const char *owner, CatalogId catalogId,
+					int subid, DumpId dumpId,
+					const char *initdb_comment)
 {
 	DumpOptions *dopt = fout->dopt;
 	CommentItem *comments;
@@ -9934,6 +9939,25 @@ dumpComment(Archive *fout, const char *type, const char *name,
 		ncomments--;
 	}
 
+	if (initdb_comment != NULL)
+	{
+		static CommentItem empty_comment = {.descr = ""};
+
+		/*
+		 * initdb creates this object with a comment.  Skip dumping the
+		 * initdb-provided comment, which would complicate matters for
+		 * non-superuser use of pg_dump.  When the DBA has removed initdb's
+		 * comment, replicate that.
+		 */
+		if (ncomments == 0)
+		{
+			comments = &empty_comment;
+			ncomments = 1;
+		}
+		else if (strcmp(comments->descr, initdb_comment) == 0)
+			ncomments = 0;
+	}
+
 	/* If a comment exists, build COMMENT ON statement */
 	if (ncomments > 0)
 	{
@@ -9969,6 +9993,21 @@ dumpComment(Archive *fout, const char *type, const char *name,
 	}
 }
 
+/*
+ * dumpComment --
+ *
+ * Typical simplification of the above function.
+ */
+static inline void
+dumpComment(Archive *fout, const char *type,
+			const char *name, const char *namespace,
+			const char *owner, CatalogId catalogId,
+			int subid, DumpId dumpId)
+{
+	dumpCommentExtended(fout, type, name, namespace, owner,
+						catalogId, subid, dumpId, NULL);
+}
+
 /*
  * dumpTableComment --
  *
@@ -10426,9 +10465,18 @@ dumpNamespace(Archive *fout, const NamespaceInfo *nspinfo)
 
 	/* Dump Schema Comments and Security Labels */
 	if (nspinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
-		dumpComment(fout, "SCHEMA", qnspname,
+	{
+		const char *initdb_comment = NULL;
+
+		if (!nspinfo->create && strcmp(qnspname, "public") == 0)
+			initdb_comment = (fout->remoteVersion >= 80300 ?
+							  "standard public schema" :
+							  "Standard public schema");
+		dumpCommentExtended(fout, "SCHEMA", qnspname,
 							NULL, nspinfo->rolname,
-					nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId);
+							nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId,
+							initdb_comment);
+	}
 
 	if (nspinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
 		dumpSecLabel(fout, "SCHEMA", qnspname,

src/bin/pg_dump/t/002_pg_dump.pl

@@ -983,9 +983,19 @@ my %tests = (
 
 	'COMMENT ON SCHEMA public' => {
 		regexp => qr/^COMMENT ON SCHEMA public IS .+;/m,
+		# regress_public_owner emits this, due to create_sql of next test
+		like => {
+			pg_dumpall_dbprivs => 1,
+			pg_dumpall_exclude => 1,
+		},
+	},
 
-		# this shouldn't ever get emitted
+	'COMMENT ON SCHEMA public IS NULL' => {
-		like => {},
+		database     => 'regress_public_owner',
+		create_order => 100,
+		create_sql   => 'COMMENT ON SCHEMA public IS NULL;',
+		regexp       => qr/^COMMENT ON SCHEMA public IS '';/m,
+		like         => { defaults_public_owner => 1 },
 	},
 
 	'COMMENT ON TABLE dump_test.test_table' => {

src/include/catalog/pg_namespace.dat

@@ -18,6 +18,7 @@
 { oid => '99', oid_symbol => 'PG_TOAST_NAMESPACE',
   descr => 'reserved schema for TOAST tables',
   nspname => 'pg_toast', nspacl => '_null_' },
+# update dumpNamespace() if changing this descr
 { oid => '2200', oid_symbol => 'PG_PUBLIC_NAMESPACE',
   descr => 'standard public schema',
   nspname => 'public', nspacl => '_null_' },