On 2025-Jun-25, Álvaro Herrera wrote:
> Yeah, I think in this case we need to extract the constraint name so
> that we have it available to print the COMMENT command, rather than
> making any assumptions about it. In fact I suspect this would fail if
> the table or column names are very long. For the other pg_dump uses of
> this logic it doesn't matter AFAIR, but here I think we must be
> stricter.
As attached.
I'm bothered by this not having any tests -- I'll see about adding some
after lunch. But at least, this seems to be dumped correctly:
CREATE TABLE supercallifragilisticexpialidocious ("dociousaliexpilistic
fragilcalirupus" int not null);
COMMENT ON CONSTRAINT "supercallifragilisticexpial_dociousaliexpilistic
fragi_not_null" ON public.supercallifragilisticexpialidocious IS 'long names,
huh?';
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
>From 0e3616d8ddc174f60a5390adedfafd699cfa966d Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
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/[email protected]
---
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
