From 0e3616d8ddc174f60a5390adedfafd699cfa966d Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 19 Jun 2025 10:26:44 +0800
Subject: [PATCH v3] fix pg_dump not dump comments on not-null constraints
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

dumpComment doesn't handle these inlined NOT NULL constraint comments, because
collectComments doesn't collect them—they aren't associated with a separate
dumpable object.  Rather than modifying collectComments, we manually dump these
inlined not-null constraint comments within dumpTableSchema.

reported by: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>

discussion: https://postgr.es/m/d50ff977-c728-4e9e-8488-fc2688e08754@oss.nttdata.com
---
 src/bin/pg_dump/pg_dump.c | 92 +++++++++++++++++++++++++++++++++++----
 src/bin/pg_dump/pg_dump.h |  1 +
 2 files changed, 84 insertions(+), 9 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index db944ec2230..5dd1f42631d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -353,6 +353,7 @@ static void determineNotNullFlags(Archive *fout, PGresult *res, int r,
 								  int i_notnull_name, int i_notnull_invalidoid,
 								  int i_notnull_noinherit,
 								  int i_notnull_islocal,
+								  int i_notnull_comment,
 								  PQExpBuffer *invalidnotnulloids);
 static char *format_function_arguments(const FuncInfo *finfo, const char *funcargs,
 									   bool is_agg);
@@ -9008,6 +9009,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_notnull_name;
 	int			i_notnull_noinherit;
 	int			i_notnull_islocal;
+	int			i_notnull_comment;
 	int			i_notnull_invalidoid;
 	int			i_attoptions;
 	int			i_attcollation;
@@ -9118,6 +9120,11 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 							 "false AS notnull_noinherit,\n"
 							 "a.attislocal AS notnull_islocal,\n");
 
+	if (fout->remoteVersion >= 180000)
+		appendPQExpBufferStr(q, "pt.description AS notnull_comment,\n");
+	else
+		appendPQExpBufferStr(q, "NULL AS notnull_comment,\n");
+
 	if (fout->remoteVersion >= 140000)
 		appendPQExpBufferStr(q,
 							 "a.attcompression AS attcompression,\n");
@@ -9158,15 +9165,19 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	/*
 	 * In versions 18 and up, we need pg_constraint for explicit NOT NULL
 	 * entries.  Also, we need to know if the NOT NULL for each column is
-	 * backing a primary key.
+	 * backing a primary key.  Lastly, we join to pg_description to get their
+	 * comments.
 	 */
 	if (fout->remoteVersion >= 180000)
+	{
 		appendPQExpBufferStr(q,
 							 " LEFT JOIN pg_catalog.pg_constraint co ON "
 							 "(a.attrelid = co.conrelid\n"
 							 "   AND co.contype = 'n' AND "
-							 "co.conkey = array[a.attnum])\n");
-
+							 "co.conkey = array[a.attnum])\n"
+							 " LEFT JOIN pg_catalog.pg_description pt ON "
+							 "(pt.classoid = co.tableoid AND pt.objoid = co.oid)\n");
+	}
 	appendPQExpBufferStr(q,
 						 "WHERE a.attnum > 0::pg_catalog.int2\n"
 						 "ORDER BY a.attrelid, a.attnum");
@@ -9192,6 +9203,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	i_notnull_invalidoid = PQfnumber(res, "notnull_invalidoid");
 	i_notnull_noinherit = PQfnumber(res, "notnull_noinherit");
 	i_notnull_islocal = PQfnumber(res, "notnull_islocal");
+	i_notnull_comment = PQfnumber(res, "notnull_comment");
 	i_attoptions = PQfnumber(res, "attoptions");
 	i_attcollation = PQfnumber(res, "attcollation");
 	i_attcompression = PQfnumber(res, "attcompression");
@@ -9260,6 +9272,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->notnull_invalid = (bool *) pg_malloc(numatts * sizeof(bool));
 		tbinfo->notnull_noinh = (bool *) pg_malloc(numatts * sizeof(bool));
 		tbinfo->notnull_islocal = (bool *) pg_malloc(numatts * sizeof(bool));
+		tbinfo->notnull_comment = (char **) pg_malloc(numatts * sizeof(char *));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *));
 		hasdefaults = false;
 
@@ -9291,8 +9304,11 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 								  i_notnull_invalidoid,
 								  i_notnull_noinherit,
 								  i_notnull_islocal,
+								  i_notnull_comment,
 								  &invalidnotnulloids);
 
+			tbinfo->notnull_comment[j] = PQgetisnull(res, r, i_notnull_comment) ?
+				NULL : pg_strdup(PQgetvalue(res, r, i_notnull_comment));
 			tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, r, i_attoptions));
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, r, i_attcollation));
 			tbinfo->attcompression[j] = *(PQgetvalue(res, r, i_attcompression));
@@ -9704,8 +9720,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
  * 4) The column has a constraint with a known name; in that case
  *    notnull_constrs carries that name and dumpTableSchema will print
  *    "CONSTRAINT the_name NOT NULL".  However, if the name is the default
- *    (table_column_not_null), there's no need to print that name in the dump,
- *    so notnull_constrs is set to the empty string and it behaves as case 2.
+ *    (table_column_not_null) and there's no comment on the constraint,
+ *    there's no need to print that name in the dump, so notnull_constrs
+ *    is set to the empty string and it behaves as case 2.
  *
  * In a child table that inherits from a parent already containing NOT NULL
  * constraints and the columns in the child don't have their own NOT NULL
@@ -9735,6 +9752,7 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
 					  int i_notnull_invalidoid,
 					  int i_notnull_noinherit,
 					  int i_notnull_islocal,
+					  int i_notnull_comment,
 					  PQExpBuffer *invalidnotnulloids)
 {
 	DumpOptions *dopt = fout->dopt;
@@ -9805,11 +9823,13 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
 		{
 			/*
 			 * In binary upgrade of inheritance child tables, must have a
-			 * constraint name that we can UPDATE later.
+			 * constraint name that we can UPDATE later; same if there's a
+			 * comment on the constraint.
 			 */
-			if (dopt->binary_upgrade &&
-				!tbinfo->ispartition &&
-				!tbinfo->notnull_islocal)
+			if ((dopt->binary_upgrade &&
+				 !tbinfo->ispartition &&
+				 !tbinfo->notnull_islocal) ||
+				!PQgetisnull(res, r, i_notnull_comment))
 			{
 				tbinfo->notnull_constrs[j] =
 					pstrdup(PQgetvalue(res, r, i_notnull_name));
@@ -17686,6 +17706,60 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 	if (tbinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
 		dumpTableSecLabel(fout, tbinfo, reltypename);
 
+	/*
+	 * Dump comments for not-null constraints that aren't to be dumped
+	 * separately (those are dumped by collectComments).
+	 */
+	if (!fout->dopt->no_comments && dopt->dumpSchema &&
+		fout->remoteVersion >= 180000)
+	{
+		const char *owner = tbinfo->rolname;
+		PQExpBuffer comment = NULL;
+		PQExpBuffer tag = NULL;
+
+		for (j = 0; j < tbinfo->numatts; j++)
+		{
+			if (tbinfo->notnull_comment[j] != NULL)
+			{
+				if (comment == NULL)
+				{
+					comment = createPQExpBuffer();
+					tag = createPQExpBuffer();
+				}
+				else
+				{
+					resetPQExpBuffer(comment);
+					resetPQExpBuffer(tag);
+				}
+
+				appendPQExpBuffer(comment, "COMMENT ON CONSTRAINT %s ",
+								  fmtId(tbinfo->notnull_constrs[j]));
+				appendPQExpBuffer(comment, "ON %s IS ", qualrelname);
+				appendStringLiteralAH(comment, tbinfo->notnull_comment[j], fout);
+				appendPQExpBufferStr(comment, ";\n");
+
+				appendPQExpBuffer(tag, "CONSTRAINT %s ON %s",
+								  fmtId(tbinfo->notnull_constrs[j]), qualrelname);
+
+				/* see dumpCommentExtended also */
+				ArchiveEntry(fout, nilCatalogId, createDumpId(),
+							 ARCHIVE_OPTS(.tag = tag->data,
+										  .namespace = tbinfo->dobj.namespace->dobj.name,
+										  .owner = owner,
+										  .description = "COMMENT",
+										  .section = SECTION_NONE,
+										  .createStmt = comment->data,
+										  .deps = &(tbinfo->dobj.dumpId),
+										  .nDeps = 1));
+			}
+		}
+		if (comment != NULL)
+		{
+			destroyPQExpBuffer(comment);
+			destroyPQExpBuffer(tag);
+		}
+	}
+
 	/* Dump comments on inlined table constraints */
 	for (j = 0; j < tbinfo->ncheck; j++)
 	{
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 7417eab6aef..39eef1d6617 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -365,6 +365,7 @@ typedef struct _tableInfo
 									 * there isn't one on this column. If
 									 * empty string, unnamed constraint
 									 * (pre-v17) */
+	char	  **notnull_comment;	/* comment thereof */
 	bool	   *notnull_invalid;	/* true for NOT NULL NOT VALID */
 	bool	   *notnull_noinh;	/* NOT NULL is NO INHERIT */
 	bool	   *notnull_islocal;	/* true if NOT NULL has local definition */
-- 
2.39.5

