From 0e3616d8ddc174f60a5390adedfafd699cfa966d Mon Sep 17 00:00:00 2001 From: jian he 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 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