On Wed, Jun 18, 2025 at 8:13 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
>
> I'm aware of a related open item [1] affecting both v17 and v18,
> but this seems like a separate issue, since it relates to a new v18 feature...
> Or we should treat them the same?
>
I think they are separate issues.
for check constraint that dump willed separately:
check constraint comments will go through dumpConstraint,
dumpTableConstraintComment
for check constraint that dump won't dump separately:
check constraint comments will go through dumpTableSchema,
dumpTableConstraintComment
Similarly we don't need to worry about not-null constraints that are
dumped separately.
dumpConstraint, dumpTableConstraintComment will do the job.
however per comment from collectComments say
"/* We needn't remember comments that don't match an */"
there is no ConstraintInfo for these inlined not-null, that means
``static CommentItem *comments = NULL;``
does not hold any comments for these inlined-null constraints.
so for
CREATE TABLE t (i int);
ALTER TABLE t ADD CONSTRAINT my_not_null NOT NULL i;
COMMENT ON CONSTRAINT my_not_null ON t IS 'my not null';
we can not locate the not-null constraint and use dumpComment to dump
the comments.
dumpComment->findComments will return nothing.
but we need to do the similar thing of dumpCommentExtended
``if (ncomments > 0){}`` code branch.
dumpTableSchema handles dumping of table column definitions, and tells us which
column print_notnull is true. Since we already know which columns have had
their not-null constraints printed, it makes sense to dump inline not-null
comments here too.
Please check the attached POC patch.
From c586c07a2c2b473f88453c12de0a08190db42a2f Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Wed, 18 Jun 2025 22:35:14 +0800
Subject: [PATCH v1 1/1] 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-fc2688e08...@oss.nttdata.com
---
src/bin/pg_dump/pg_dump.c | 111 ++++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7bc0724cd30..909ce30d553 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16769,6 +16769,9 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
char *storage;
int j,
k;
+ bool *notnullprinted;
+ bool notnull_printed = false; /* at least one not-null printed */
+ notnullprinted = (bool *) pg_malloc0(tbinfo->numatts * sizeof(bool));
/* We had better have loaded per-column details about this table */
Assert(tbinfo->interesting);
@@ -17016,6 +17019,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
if (print_notnull)
{
+ notnullprinted[j] = true;
if (tbinfo->notnull_constrs[j][0] == '\0')
appendPQExpBufferStr(q, " NOT NULL");
else
@@ -17684,6 +17688,113 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
if (tbinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
dumpTableSecLabel(fout, tbinfo, reltypename);
+ /*
+ * Dump Table not-null constraint comments.
+ * Invalid not-null constraints are dumped separately. So we only need to
+ * handle comments for these "inlined" not-null constraints.
+ * collectComments function does not collect comments for inlined not-null
+ * constraints, as they don't have separate dumpable object associated with
+ * it. Therefore, we need to explicitly dump comments for these "inlined"
+ * not-null constraints now, since they won't be dumped separately.
+ */
+ for (j = 0; j < tbinfo->numatts; j++)
+ {
+ if (notnullprinted[j])
+ {
+ notnull_printed = true;
+ break;
+ }
+ }
+
+ if (!fout->dopt->no_comments &&
+ dopt->dumpSchema &&
+ fout->remoteVersion >= 180000 &&
+ notnull_printed)
+ {
+ PGresult *res;
+ int i_description;
+ int i_constrname;
+ bool firstitem = true;
+ char *qtabname;
+ const char *descr;
+ const char *constrname;
+ const char *namespace = tbinfo->dobj.namespace->dobj.name;
+ const char *owner = tbinfo->rolname;
+ PQExpBuffer query = createPQExpBuffer();
+ PQExpBuffer comments = createPQExpBuffer();
+ PQExpBuffer conprefix = createPQExpBuffer();
+ PQExpBuffer tag = createPQExpBuffer();
+ DumpId dumpId = tbinfo->dobj.dumpId;
+
+ qtabname = pg_strdup(fmtId(tbinfo->dobj.name));
+ appendPQExpBufferStr(query, "SELECT pt.description, pc.conname\n"
+ "FROM pg_catalog.pg_constraint pc\n"
+ "JOIN pg_catalog.pg_description pt\n"
+ "ON pt.classoid = pc.tableoid AND pc.oid = pt.objoid\n"
+ "WHERE contype = 'n' AND conrelid = ");
+ appendStringLiteralAH(query, fmtQualifiedDumpable(tbinfo), fout);
+
+ appendPQExpBufferStr(query, "::pg_catalog.regclass AND conkey IN (");
+ for (j = 0; j < tbinfo->numatts; j++)
+ {
+ if (notnullprinted[j])
+ {
+ Assert(tbinfo->notnull_constrs[j] != NULL);
+
+ if (firstitem)
+ firstitem = false;
+ else
+ appendPQExpBufferStr(query, ", ");
+
+ appendPQExpBuffer(query, "'{%d}'", j + 1);
+ }
+ }
+ appendPQExpBufferChar(query, ')');
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ i_description = PQfnumber(res, "description");
+ i_constrname = PQfnumber(res, "conname");
+
+ for (int i = 0; i < PQntuples(res); i++)
+ {
+ descr = PQgetvalue(res, i, i_description);
+ constrname = PQgetvalue(res, i, i_constrname);
+
+ appendPQExpBuffer(conprefix, "CONSTRAINT %s ON",
+ fmtId(constrname));
+
+ appendPQExpBuffer(comments, "COMMENT ON %s ", conprefix->data);
+ if (namespace && *namespace)
+ appendPQExpBuffer(comments, "%s.", fmtId(namespace));
+ appendPQExpBuffer(comments, "%s IS ", qtabname);
+ appendStringLiteralAH(comments, descr, fout);
+ appendPQExpBufferStr(comments, ";\n");
+
+ appendPQExpBuffer(tag, "%s %s", conprefix->data, qtabname);
+
+ /* see dumpCommentExtended comments also */
+ ArchiveEntry(fout, nilCatalogId, createDumpId(),
+ ARCHIVE_OPTS(.tag = tag->data,
+ .namespace = namespace,
+ .owner = owner,
+ .description = "COMMENT",
+ .section = SECTION_NONE,
+ .createStmt = comments->data,
+ .deps = &dumpId,
+ .nDeps = 1));
+
+ resetPQExpBuffer(comments);
+ resetPQExpBuffer(tag);
+ resetPQExpBuffer(conprefix);
+ }
+ PQclear(res);
+ destroyPQExpBuffer(query);
+ destroyPQExpBuffer(conprefix);
+ destroyPQExpBuffer(comments);
+ destroyPQExpBuffer(tag);
+ free(qtabname);
+ }
+
/* Dump comments on inlined table constraints */
for (j = 0; j < tbinfo->ncheck; j++)
{
--
2.34.1
