pg_dump misses comments on NOT NULL constraints
Hi,
In v18, we can now add comments on NOT NULL constraints. However, I noticed
that pg_dump doesn't include those comments in its output. For example:
--------------------------
$ psql <<EOF
CREATE TABLE t (i int);
ALTER TABLE t ADD CONSTRAINT my_not_null NOT NULL i;
ALTER TABLE t ADD CONSTRAINT my_check CHECK (i > 0);
COMMENT ON CONSTRAINT my_not_null ON t IS 'my not null';
COMMENT ON CONSTRAINT my_check ON t IS 'my check';
EOF
$ pg_dump | grep COMMENT
-- Name: CONSTRAINT my_check ON t; Type: COMMENT; Schema: public; Owner: postgres
COMMENT ON CONSTRAINT my_check ON public.t IS 'my check';
--------------------------
As shown above, the comment on my_not_null is missing from the dump output.
Is this an oversight in commit 14e87ffa5c5? If so, I'll add it as
a v18 open item.
I'm aware of a related open item [1]/messages/by-id/CACJufxF-0bqVR=j4jonS6N2Ka6hHUpFyu3_3TWKNhOW_4yFSSg@mail.gmail.com 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?
Regards,
[1]: /messages/by-id/CACJufxF-0bqVR=j4jonS6N2Ka6hHUpFyu3_3TWKNhOW_4yFSSg@mail.gmail.com
--
Fujii Masao
NTT DATA Japan Corporation
On 2025/06/18 9:13, Fujii Masao wrote:
Hi,
In v18, we can now add comments on NOT NULL constraints. However, I noticed
that pg_dump doesn't include those comments in its output. For example:--------------------------
$ psql <<EOF
CREATE TABLE t (i int);
ALTER TABLE t ADD CONSTRAINT my_not_null NOT NULL i;
ALTER TABLE t ADD CONSTRAINT my_check CHECK (i > 0);
COMMENT ON CONSTRAINT my_not_null ON t IS 'my not null';
COMMENT ON CONSTRAINT my_check ON t IS 'my check';
EOF$ pg_dump | grep COMMENT
-- Name: CONSTRAINT my_check ON t; Type: COMMENT; Schema: public; Owner: postgres
COMMENT ON CONSTRAINT my_check ON public.t IS 'my check';
--------------------------As shown above, the comment on my_not_null is missing from the dump output.
Is this an oversight in commit 14e87ffa5c5? If so, I'll add it as
a v18 open item.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 ran into another issue related to comments on NOT NULL constraints.
When using CREATE TABLE ... (LIKE ... INCLUDING ALL), the NOT NULL constraints
are copied, but their comments are not. For example:
-----------------------------------------------------
=# CREATE TABLE t (i int);
=# ALTER TABLE t ADD CONSTRAINT my_not_null_i NOT NULL i;
=# ALTER TABLE t ADD CONSTRAINT my_check_i CHECK (i > 0);
=# COMMENT ON CONSTRAINT my_not_null_i ON t IS 'my not null for i';
=# COMMENT ON CONSTRAINT my_check_i ON t IS 'my check for i';
=# CREATE TABLE t_copied (LIKE t INCLUDING ALL);
=# SELECT cls.relname, cnst.conname, obj_description(cnst.oid, 'pg_constraint')
FROM pg_constraint cnst, pg_class cls
WHERE cnst.conrelid = cls.oid AND cnst.conname like '%my_%'
ORDER BY cls.relname, cnst.conname;
relname | conname | obj_description
----------+---------------+-------------------
t | my_check_i | my check for i
t | my_not_null_i | my not null for i
t_copied | my_check_i | my check for i
t_copied | my_not_null_i | (null)
(4 rows)
-----------------------------------------------------
As shown, the comment on my_not_null_i is not copied to the new table,
even though the constraint itself is. Could this be another oversight
in commit 14e87ffa5c5?
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025-Jun-18, Fujii Masao wrote:
On 2025/06/18 9:13, Fujii Masao wrote:
In v18, we can now add comments on NOT NULL constraints. However, I noticed
that pg_dump doesn't include those comments in its output. For example:
This is definitely a bug, thanks for reporting.
I ran into another issue related to comments on NOT NULL constraints.
When using CREATE TABLE ... (LIKE ... INCLUDING ALL), the NOT NULL constraints
are copied, but their comments are not.
... and I agree that it'd be good to fix this one as well.
Jian He just mentioned to me that he's looking into these problems.
Many thanks to him. I'm pretty much unable to get anything done this
week, but I'll be back online next week.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Java is clearly an example of money oriented programming" (A. Stepanov)
On Wed, Jun 18, 2025 at 8:13 AM Fujii Masao <masao.fujii@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.
Attachments:
v1-0001-fix-pg_dump-not-dump-comments-on-not-null-constraints.patchtext/x-patch; charset=UTF-8; name=v1-0001-fix-pg_dump-not-dump-comments-on-not-null-constraints.patchDownload
From c586c07a2c2b473f88453c12de0a08190db42a2f Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@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-fc2688e08754@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
On 2025-Jun-18, jian he wrote:
Similarly we don't need to worry about not-null constraints that are
dumped separately.
dumpConstraint, dumpTableConstraintComment will do the job.
Right.
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.
I agree that this is roughly the right approach, but I think you're
doing it harder than it needs to be -- it might be easier to add a JOIN
to pg_description to the big query in getTableAttrs(), and add a pointer
to the returned string in tbinfo->notnull_comments[j] (for versions
earlier than 18, don't add the join and have it return constant NULL).
Then in dumpTableSchema, in the place where you added the new query,
just scan that array and print COMMENT ON commands for each valid
constraint where that's not a null pointer.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)
On Wed, Jun 18, 2025 at 11:05 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
I agree that this is roughly the right approach, but I think you're
doing it harder than it needs to be -- it might be easier to add a JOIN
to pg_description to the big query in getTableAttrs(), and add a pointer
to the returned string in tbinfo->notnull_comments[j] (for versions
earlier than 18, don't add the join and have it return constant NULL).
Then in dumpTableSchema, in the place where you added the new query,
just scan that array and print COMMENT ON commands for each valid
constraint where that's not a null pointer.
Previously I was worried about print_notnull, shouldPrintColumn.
if there is a not-null constraint that is not dumped separately, it has comments
then we should dump these comments, then no need to worry about print_notnull.
using notnull_comments saves us one more query.
however, in determineNotNullFlags we have:
char *default_name;
/* XXX should match ChooseConstraintName better */
default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
tbinfo->attnames[j]);
if (strcmp(default_name,
PQgetvalue(res, r, i_notnull_name)) == 0)
tbinfo->notnull_constrs[j] = "";
then we can not blindly use tbinfo->notnull_constrs as the not-null
constraint name.
if tbinfo->notnull_constrs is an empty string, we need to use the above
"%s_%s_not_null" trick to get the default no-null constraint name.
Attachments:
v2-0001-fix-pg_dump-not-dump-comments-on-not-null-constraints.patchtext/x-patch; charset=UTF-8; name=v2-0001-fix-pg_dump-not-dump-comments-on-not-null-constraints.patchDownload
From 4cce00779490001f4e40fb3055c7bddd539e1ad2 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 v2 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-fc2688e08754@oss.nttdata.com
---
src/bin/pg_dump/pg_dump.c | 95 ++++++++++++++++++++++++++++++++++++++-
src/bin/pg_dump/pg_dump.h | 1 +
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7bc0724cd30..dbd557b7484 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9006,6 +9006,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
int i_notnull_name;
int i_notnull_noinherit;
int i_notnull_islocal;
+ int i_notnull_comments;
int i_notnull_invalidoid;
int i_attoptions;
int i_attcollation;
@@ -9116,6 +9117,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_comments,\n");
+ else
+ appendPQExpBufferStr(q, "NULL AS notnull_comments,\n");
+
if (fout->remoteVersion >= 140000)
appendPQExpBufferStr(q,
"a.attcompression AS attcompression,\n");
@@ -9159,12 +9165,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
* backing a primary key.
*/
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");
-
+ appendPQExpBufferStr(q,
+ " 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");
@@ -9190,6 +9200,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_comments = PQfnumber(res, "notnull_comments");
i_attoptions = PQfnumber(res, "attoptions");
i_attcollation = PQfnumber(res, "attcollation");
i_attcompression = PQfnumber(res, "attcompression");
@@ -9258,6 +9269,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_comments = (char **) pg_malloc(numatts * sizeof(char *));
tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *));
hasdefaults = false;
@@ -9291,6 +9303,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
i_notnull_islocal,
&invalidnotnulloids);
+ if (PQgetisnull(res, r, i_notnull_comments))
+ tbinfo->notnull_comments[j] = NULL;
+ else
+ tbinfo->notnull_comments[j] = pg_strdup(PQgetvalue(res, r, i_notnull_comments));
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));
@@ -17684,6 +17700,83 @@ 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.
+ */
+ if (!fout->dopt->no_comments &&
+ dopt->dumpSchema &&
+ fout->remoteVersion >= 180000)
+ {
+ char *qtabname;
+ const char *namespace = tbinfo->dobj.namespace->dobj.name;
+ const char *owner = tbinfo->rolname;
+ PQExpBuffer comments = createPQExpBuffer();
+ PQExpBuffer conprefix = createPQExpBuffer();
+ PQExpBuffer tag = createPQExpBuffer();
+ DumpId dumpId = tbinfo->dobj.dumpId;
+ qtabname = pg_strdup(fmtId(tbinfo->dobj.name));
+
+ for (j = 0; j < tbinfo->numatts; j++)
+ {
+ if (tbinfo->notnull_constrs[j] != NULL &&
+ tbinfo->notnull_comments[j] != NULL)
+ {
+ /*
+ * For some not-null constraints, the notnull_constrs is an
+ * empty string, because in certain cases we don't need to emit
+ * the constraint name. See determineNotNullFlags too.
+ */
+ if (tbinfo->notnull_constrs[j][0] == '\0')
+ {
+ char *default_name;
+ default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
+ tbinfo->attnames[j]);
+
+ appendPQExpBuffer(conprefix, "CONSTRAINT %s ON",
+ fmtId(default_name));
+ }
+ else
+ appendPQExpBuffer(conprefix, "CONSTRAINT %s ON",
+ fmtId(tbinfo->notnull_constrs[j]));
+
+ appendPQExpBuffer(comments, "COMMENT ON %s ", conprefix->data);
+ if (namespace && *namespace)
+ appendPQExpBuffer(comments, "%s.", fmtId(namespace));
+
+ appendPQExpBuffer(comments, "%s IS ", qtabname);
+ appendStringLiteralAH(comments, tbinfo->notnull_comments[j], fout);
+ appendPQExpBufferStr(comments, ";\n");
+
+ appendPQExpBuffer(tag, "%s %s", conprefix->data, qtabname);
+
+ /* see dumpCommentExtended 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);
+ }
+ }
+ destroyPQExpBuffer(conprefix);
+ destroyPQExpBuffer(comments);
+ destroyPQExpBuffer(tag);
+ free(qtabname);
+ }
+
/* 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..8092d5fddc2 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -368,6 +368,7 @@ typedef struct _tableInfo
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 */
+ char **notnull_comments; /* per-attribute not-null constraint's comments */
struct _attrDefInfo **attrdefs; /* DEFAULT expressions */
struct _constraintInfo *checkexprs; /* CHECK constraints */
struct _relStatsInfo *stats; /* only set for matviews */
--
2.34.1
On Wed, Jun 18, 2025 at 10:21 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
I ran into another issue related to comments on NOT NULL constraints.
When using CREATE TABLE ... (LIKE ... INCLUDING ALL), the NOT NULL constraints
are copied, but their comments are not. For example:-----------------------------------------------------
=# CREATE TABLE t (i int);
=# ALTER TABLE t ADD CONSTRAINT my_not_null_i NOT NULL i;
=# ALTER TABLE t ADD CONSTRAINT my_check_i CHECK (i > 0);
=# COMMENT ON CONSTRAINT my_not_null_i ON t IS 'my not null for i';
=# COMMENT ON CONSTRAINT my_check_i ON t IS 'my check for i';=# CREATE TABLE t_copied (LIKE t INCLUDING ALL);
As shown, the comment on my_not_null_i is not copied to the new table,
even though the constraint itself is. Could this be another oversight
in commit 14e87ffa5c5?
hi.
in transformTableLikeClausem, let cxt(CreateStmtContext) to add
CommentStmt should just work.
Please check attached, tests also added.
Attachments:
v1-0001-fix-create-table-like-not-copy-not-null-constraint-s-comm.patchapplication/x-patch; name=v1-0001-fix-create-table-like-not-copy-not-null-constraint-s-comm.patchDownload
From 41fd109bd75451cadc347cc37be56228e85f3417 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 19 Jun 2025 13:40:06 +0800
Subject: [PATCH v1 1/1] fix create table like not copy not null constraint's
comments
We copy the source table's not-null constraints unconditionally.
when using CREATE TABLE ... LIKE (INCLUDING COMMENT), the comments on the source
table's not-null constraints should also be copied to the new destination table.
this is a oversight of commit 14e87ffa5c5([1])
reported by: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
[1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=14e87ffa5c543b5f30ead7413084c25f7735039f
discussion: https://postgr.es/m/127debef-e558-4784-9e24-0d5eaf91e2d1@oss.nttdata.com
---
src/backend/parser/parse_utilcmd.c | 23 +++++++++++++++++++
.../regress/expected/create_table_like.out | 15 +++++++++++-
src/test/regress/sql/create_table_like.sql | 12 +++++++++-
3 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 62015431fdf..2bd18c597d7 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1278,6 +1278,29 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
true);
+
+ /* Copy comments on not-null constraints */
+ if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
+ {
+ foreach_node(Constraint, nnconstr, lst)
+ {
+ if ((comment = GetComment(get_relation_constraint_oid(RelationGetRelid(relation),
+ nnconstr->conname, false),
+ ConstraintRelationId,
+ 0)) != NULL)
+ {
+ CommentStmt *stmt = makeNode(CommentStmt);
+
+ stmt->objtype = OBJECT_TABCONSTRAINT;
+ stmt->object = (Node *) list_make3(makeString(cxt->relation->schemaname),
+ makeString(cxt->relation->relname),
+ makeString(nnconstr->conname));
+ stmt->comment = comment;
+ cxt->alist = lappend(cxt->alist, stmt);
+ }
+ }
+ }
+
cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
}
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index bf34289e984..1374a972e6e 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -529,7 +529,9 @@ NOTICE: drop cascades to table inhe
-- LIKE must respect NO INHERIT property of constraints
CREATE TABLE noinh_con_copy (a int CHECK (a > 0) NO INHERIT, b int not null,
c int not null no inherit);
-CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS);
+COMMENT ON CONSTRAINT noinh_con_copy_b_not_null ON noinh_con_copy IS 'not null b';
+COMMENT ON CONSTRAINT noinh_con_copy_c_not_null ON noinh_con_copy IS 'not null c no inherit';
+CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS INCLUDING COMMENTS);
\d+ noinh_con_copy1
Table "public.noinh_con_copy1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
@@ -543,6 +545,17 @@ Not-null constraints:
"noinh_con_copy_b_not_null" NOT NULL "b"
"noinh_con_copy_c_not_null" NOT NULL "c" NO INHERIT
+SELECT conname, description
+FROM pg_description, pg_constraint c
+WHERE classoid = 'pg_constraint'::regclass
+AND objoid = c.oid AND c.conrelid = 'noinh_con_copy1'::regclass
+ORDER BY conname COLLATE "C";
+ conname | description
+---------------------------+-----------------------
+ noinh_con_copy_b_not_null | not null b
+ noinh_con_copy_c_not_null | not null c no inherit
+(2 rows)
+
-- fail, as partitioned tables don't allow NO INHERIT constraints
CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
PARTITION BY LIST (a);
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 6e21722aaeb..6da7f4f0557 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -197,9 +197,19 @@ DROP TABLE ctlt1, ctlt2, ctlt3, ctlt4, ctlt12_storage, ctlt12_comments, ctlt1_in
-- LIKE must respect NO INHERIT property of constraints
CREATE TABLE noinh_con_copy (a int CHECK (a > 0) NO INHERIT, b int not null,
c int not null no inherit);
-CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS);
+
+COMMENT ON CONSTRAINT noinh_con_copy_b_not_null ON noinh_con_copy IS 'not null b';
+COMMENT ON CONSTRAINT noinh_con_copy_c_not_null ON noinh_con_copy IS 'not null c no inherit';
+
+CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS INCLUDING COMMENTS);
\d+ noinh_con_copy1
+SELECT conname, description
+FROM pg_description, pg_constraint c
+WHERE classoid = 'pg_constraint'::regclass
+AND objoid = c.oid AND c.conrelid = 'noinh_con_copy1'::regclass
+ORDER BY conname COLLATE "C";
+
-- fail, as partitioned tables don't allow NO INHERIT constraints
CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
PARTITION BY LIST (a);
--
2.34.1
On 2025/06/19 14:42, jian he wrote:
On Wed, Jun 18, 2025 at 10:21 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:I ran into another issue related to comments on NOT NULL constraints.
When using CREATE TABLE ... (LIKE ... INCLUDING ALL), the NOT NULL constraints
are copied, but their comments are not. For example:-----------------------------------------------------
=# CREATE TABLE t (i int);
=# ALTER TABLE t ADD CONSTRAINT my_not_null_i NOT NULL i;
=# ALTER TABLE t ADD CONSTRAINT my_check_i CHECK (i > 0);
=# COMMENT ON CONSTRAINT my_not_null_i ON t IS 'my not null for i';
=# COMMENT ON CONSTRAINT my_check_i ON t IS 'my check for i';=# CREATE TABLE t_copied (LIKE t INCLUDING ALL);
As shown, the comment on my_not_null_i is not copied to the new table,
even though the constraint itself is. Could this be another oversight
in commit 14e87ffa5c5?hi.
in transformTableLikeClausem, let cxt(CreateStmtContext) to add
CommentStmt should just work.
Please check attached, tests also added.
Thanks for the patch! LGTM.
Just one minor suggestion:
+ /* Copy comments on not-null constraints */
+ if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
+ {It might be clearer to move this block after the line:
cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
That would make the code a bit more readable.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025/06/19 11:38, jian he wrote:
On Wed, Jun 18, 2025 at 11:05 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
I agree that this is roughly the right approach, but I think you're
doing it harder than it needs to be -- it might be easier to add a JOIN
to pg_description to the big query in getTableAttrs(), and add a pointer
to the returned string in tbinfo->notnull_comments[j] (for versions
earlier than 18, don't add the join and have it return constant NULL).
Then in dumpTableSchema, in the place where you added the new query,
just scan that array and print COMMENT ON commands for each valid
constraint where that's not a null pointer.Previously I was worried about print_notnull, shouldPrintColumn.
if there is a not-null constraint that is not dumped separately, it has comments
then we should dump these comments, then no need to worry about print_notnull.using notnull_comments saves us one more query.
however, in determineNotNullFlags we have:char *default_name;
/* XXX should match ChooseConstraintName better */
default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
tbinfo->attnames[j]);
if (strcmp(default_name,
PQgetvalue(res, r, i_notnull_name)) == 0)
tbinfo->notnull_constrs[j] = "";
Do we really need this part? With the patch, due to this part,
pg_dump might emit output like:
CREATE TABLE t (i int NOT NULL);
COMMENT ON CONSTRAINT t_i_not_null ON public.t IS '.......';
This seems to rely on the assumption that the naming convention for
NOT NULL constraints (e.g., t_i_not_null) will remain stable across versions.
If that ever changes, the dumped DDL could fail.
Wouldn't it be safer to always print the constraint name explicitly,
even if it's the default name? For example:
CREATE TABLE t (i int CONSTRAINT t_i_not_null NOT NULL);
COMMENT ON CONSTRAINT t_i_not_null ON public.t IS '.......';
This is consistent with how check constraints are handled.
Regarding the patch:
+ if (!fout->dopt->no_comments &&
+ dopt->dumpSchema &&
+ fout->remoteVersion >= 180000)The dopt->dumpSchema check is redundant, since dumpTableSchema() is
only called when that flag is already true?
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025/06/19 20:53, Fujii Masao wrote:
On 2025/06/19 14:42, jian he wrote:
On Wed, Jun 18, 2025 at 10:21 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:I ran into another issue related to comments on NOT NULL constraints.
When using CREATE TABLE ... (LIKE ... INCLUDING ALL), the NOT NULL constraints
are copied, but their comments are not. For example:-----------------------------------------------------
=# CREATE TABLE t (i int);
=# ALTER TABLE t ADD CONSTRAINT my_not_null_i NOT NULL i;
=# ALTER TABLE t ADD CONSTRAINT my_check_i CHECK (i > 0);
=# COMMENT ON CONSTRAINT my_not_null_i ON t IS 'my not null for i';
=# COMMENT ON CONSTRAINT my_check_i ON t IS 'my check for i';=# CREATE TABLE t_copied (LIKE t INCLUDING ALL);
As shown, the comment on my_not_null_i is not copied to the new table,
even though the constraint itself is. Could this be another oversight
in commit 14e87ffa5c5?hi.
in transformTableLikeClausem, let cxt(CreateStmtContext) to add
CommentStmt should just work.
Please check attached, tests also added.Thanks for the patch! LGTM.
Just one minor suggestion:
+ /* Copy comments on not-null constraints */ + if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS) + {It might be clearer to move this block after the line:
cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
That would make the code a bit more readable.
I've applied this cosmetic change to the patch.
Barring objections, I'll commit the patch.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
Attachments:
v2-0001-Make-CREATE-TABLE-LIKE-copy-comments-on-NOT-NULL-.patchtext/plain; charset=UTF-8; name=v2-0001-Make-CREATE-TABLE-LIKE-copy-comments-on-NOT-NULL-.patchDownload
From 516e647e7d1fdafc64dba092389963f32cd688e5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 25 Jun 2025 10:02:56 +0900
Subject: [PATCH v2] Make CREATE TABLE LIKE copy comments on NOT NULL
constraints when requested.
Commit 14e87ffa5c5 introduced support for adding comments to NOT NULL
constraints. However, CREATE TABLE LIKE INCLUDING COMMENTS did not copy
these comments to the new table. This was an oversight in that commit.
This commit corrects the behavior by ensuring CREATE TABLE LIKE to also copy
the comments on NOT NULL constraints when INCLUDING COMMENTS is specified.
Author: Jian He <jian.universality@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/127debef-e558-4784-9e24-0d5eaf91e2d1@oss.nttdata.com
---
src/backend/parser/parse_utilcmd.c | 22 +++++++++++++++++++
.../regress/expected/create_table_like.out | 15 ++++++++++++-
src/test/regress/sql/create_table_like.sql | 12 +++++++++-
3 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 62015431fdf..afcf54169c3 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1279,6 +1279,28 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
true);
cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
+
+ /* Copy comments on not-null constraints */
+ if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
+ {
+ foreach_node(Constraint, nnconstr, lst)
+ {
+ if ((comment = GetComment(get_relation_constraint_oid(RelationGetRelid(relation),
+ nnconstr->conname, false),
+ ConstraintRelationId,
+ 0)) != NULL)
+ {
+ CommentStmt *stmt = makeNode(CommentStmt);
+
+ stmt->objtype = OBJECT_TABCONSTRAINT;
+ stmt->object = (Node *) list_make3(makeString(cxt->relation->schemaname),
+ makeString(cxt->relation->relname),
+ makeString(nnconstr->conname));
+ stmt->comment = comment;
+ cxt->alist = lappend(cxt->alist, stmt);
+ }
+ }
+ }
}
/*
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index bf34289e984..1374a972e6e 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -529,7 +529,9 @@ NOTICE: drop cascades to table inhe
-- LIKE must respect NO INHERIT property of constraints
CREATE TABLE noinh_con_copy (a int CHECK (a > 0) NO INHERIT, b int not null,
c int not null no inherit);
-CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS);
+COMMENT ON CONSTRAINT noinh_con_copy_b_not_null ON noinh_con_copy IS 'not null b';
+COMMENT ON CONSTRAINT noinh_con_copy_c_not_null ON noinh_con_copy IS 'not null c no inherit';
+CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS INCLUDING COMMENTS);
\d+ noinh_con_copy1
Table "public.noinh_con_copy1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
@@ -543,6 +545,17 @@ Not-null constraints:
"noinh_con_copy_b_not_null" NOT NULL "b"
"noinh_con_copy_c_not_null" NOT NULL "c" NO INHERIT
+SELECT conname, description
+FROM pg_description, pg_constraint c
+WHERE classoid = 'pg_constraint'::regclass
+AND objoid = c.oid AND c.conrelid = 'noinh_con_copy1'::regclass
+ORDER BY conname COLLATE "C";
+ conname | description
+---------------------------+-----------------------
+ noinh_con_copy_b_not_null | not null b
+ noinh_con_copy_c_not_null | not null c no inherit
+(2 rows)
+
-- fail, as partitioned tables don't allow NO INHERIT constraints
CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
PARTITION BY LIST (a);
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 6e21722aaeb..6da7f4f0557 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -197,9 +197,19 @@ DROP TABLE ctlt1, ctlt2, ctlt3, ctlt4, ctlt12_storage, ctlt12_comments, ctlt1_in
-- LIKE must respect NO INHERIT property of constraints
CREATE TABLE noinh_con_copy (a int CHECK (a > 0) NO INHERIT, b int not null,
c int not null no inherit);
-CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS);
+
+COMMENT ON CONSTRAINT noinh_con_copy_b_not_null ON noinh_con_copy IS 'not null b';
+COMMENT ON CONSTRAINT noinh_con_copy_c_not_null ON noinh_con_copy IS 'not null c no inherit';
+
+CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING CONSTRAINTS INCLUDING COMMENTS);
\d+ noinh_con_copy1
+SELECT conname, description
+FROM pg_description, pg_constraint c
+WHERE classoid = 'pg_constraint'::regclass
+AND objoid = c.oid AND c.conrelid = 'noinh_con_copy1'::regclass
+ORDER BY conname COLLATE "C";
+
-- fail, as partitioned tables don't allow NO INHERIT constraints
CREATE TABLE noinh_con_copy1_parted (LIKE noinh_con_copy INCLUDING ALL)
PARTITION BY LIST (a);
--
2.49.0
On 2025-Jun-25, Fujii Masao wrote:
From 516e647e7d1fdafc64dba092389963f32cd688e5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 25 Jun 2025 10:02:56 +0900
Subject: [PATCH v2] Make CREATE TABLE LIKE copy comments on NOT NULL
constraints when requested.Commit 14e87ffa5c5 introduced support for adding comments to NOT NULL
constraints. However, CREATE TABLE LIKE INCLUDING COMMENTS did not copy
these comments to the new table. This was an oversight in that commit.This commit corrects the behavior by ensuring CREATE TABLE LIKE to also copy
the comments on NOT NULL constraints when INCLUDING COMMENTS is specified.
LGTM. I'd add a line in the test showing that these comments are copied
even if INCLUDING CONSTRAINTS is not given, because not-null constraints
comments themselves are -- as attached.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"It takes less than 2 seconds to get to 78% complete; that's a good sign.
A few seconds later it's at 90%, but it seems to have stuck there. Did
somebody make percentages logarithmic while I wasn't looking?"
http://smylers.hates-software.com/2005/09/08/1995c749.html
Attachments:
0001-test-fixup.patch.txttext/plain; charset=utf-8Download
From 3a5b66feaf4824707f5e09f084ed7b303589f887 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 25 Jun 2025 13:42:03 +0200
Subject: [PATCH] test fixup
---
src/test/regress/expected/create_table_like.out | 12 +++++++-----
src/test/regress/sql/create_table_like.sql | 3 ++-
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 1374a972e6e..7092b42672d 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -329,6 +329,7 @@ COMMENT ON STATISTICS ctlt1_expr_stat IS 'ab expr stats';
COMMENT ON COLUMN ctlt1.a IS 'A';
COMMENT ON COLUMN ctlt1.b IS 'B';
COMMENT ON CONSTRAINT ctlt1_a_check ON ctlt1 IS 't1_a_check';
+COMMENT ON CONSTRAINT ctlt1_a_not_null ON ctlt1 IS 't1_a_notnull';
COMMENT ON INDEX ctlt1_pkey IS 'index pkey';
COMMENT ON INDEX ctlt1_b_key IS 'index b_key';
ALTER TABLE ctlt1 ALTER COLUMN a SET STORAGE MAIN;
@@ -384,11 +385,12 @@ Not-null constraints:
"ctlt1_a_not_null" NOT NULL "a" (local, inherited)
Inherits: ctlt1
-SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass;
- description
--------------
- t1_a_check
-(1 row)
+SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";
+ conname | description
+------------------+--------------
+ ctlt1_a_check | t1_a_check
+ ctlt1_a_not_null | t1_a_notnull
+(2 rows)
CREATE TABLE ctlt13_inh () INHERITS (ctlt1, ctlt3);
NOTICE: merging multiple inherited definitions of column "a"
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 6da7f4f0557..9edde9830e7 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -139,6 +139,7 @@ COMMENT ON STATISTICS ctlt1_expr_stat IS 'ab expr stats';
COMMENT ON COLUMN ctlt1.a IS 'A';
COMMENT ON COLUMN ctlt1.b IS 'B';
COMMENT ON CONSTRAINT ctlt1_a_check ON ctlt1 IS 't1_a_check';
+COMMENT ON CONSTRAINT ctlt1_a_not_null ON ctlt1 IS 't1_a_notnull';
COMMENT ON INDEX ctlt1_pkey IS 'index pkey';
COMMENT ON INDEX ctlt1_b_key IS 'index b_key';
ALTER TABLE ctlt1 ALTER COLUMN a SET STORAGE MAIN;
@@ -164,7 +165,7 @@ CREATE TABLE ctlt12_comments (LIKE ctlt1 INCLUDING COMMENTS, LIKE ctlt2 INCLUDIN
\d+ ctlt12_comments
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1);
\d+ ctlt1_inh
-SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass;
+SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";
CREATE TABLE ctlt13_inh () INHERITS (ctlt1, ctlt3);
\d+ ctlt13_inh
CREATE TABLE ctlt13_like (LIKE ctlt3 INCLUDING CONSTRAINTS INCLUDING INDEXES INCLUDING COMMENTS INCLUDING STORAGE) INHERITS (ctlt1);
--
2.39.5
On 2025-Jun-25, Fujii Masao wrote:
however, in determineNotNullFlags we have:
char *default_name;
/* XXX should match ChooseConstraintName better */
default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
tbinfo->attnames[j]);
if (strcmp(default_name,
PQgetvalue(res, r, i_notnull_name)) == 0)
tbinfo->notnull_constrs[j] = "";
Do we really need this part? With the patch, due to this part,
pg_dump might emit output like:CREATE TABLE t (i int NOT NULL);
COMMENT ON CONSTRAINT t_i_not_null ON public.t IS '.......';This seems to rely on the assumption that the naming convention for
NOT NULL constraints (e.g., t_i_not_null) will remain stable across versions.
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.
Wouldn't it be safer to always print the constraint name explicitly,
even if it's the default name?
Yes, but we wanted to avoid printing those names in "normal cases," so
we go some lengths to avoid them if not necessary. However, I suspect
it's easy to print the names only if a comment exists -- which is also
an unusual case.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"XML!" Exclaimed C++. "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.
https://burningbird.net/the-parable-of-the-languages/
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/
Attachments:
v3-0001-fix-pg_dump-not-dump-comments-on-not-null-constra.patchtext/x-diff; charset=utf-8Download
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
On 2025/06/25 20:46, Álvaro Herrera wrote:
On 2025-Jun-25, Fujii Masao wrote:
From 516e647e7d1fdafc64dba092389963f32cd688e5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Wed, 25 Jun 2025 10:02:56 +0900
Subject: [PATCH v2] Make CREATE TABLE LIKE copy comments on NOT NULL
constraints when requested.Commit 14e87ffa5c5 introduced support for adding comments to NOT NULL
constraints. However, CREATE TABLE LIKE INCLUDING COMMENTS did not copy
these comments to the new table. This was an oversight in that commit.This commit corrects the behavior by ensuring CREATE TABLE LIKE to also copy
the comments on NOT NULL constraints when INCLUDING COMMENTS is specified.LGTM. I'd add a line in the test showing that these comments are copied
even if INCLUDING CONSTRAINTS is not given,
+1 to adding that test.
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1);
\d+ ctlt1_inh
-SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass;
+SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";However, since ctlt1_inh is created with INCLUDING COMMENTS, this test
doesn't seem to demonstrate the case you mentioned — that comments on
not-null constraints are copied even without INCLUDING CONSTRAINTS.
Am I misunderstanding?
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025/06/25 22:36, Álvaro Herrera wrote:
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.
Thanks for the patch! I agree with the approach, i.e., printing the not-null
constraint name only when there's a comment on it.
However, with the patch applied, I encountered a segmentation fault in pg_dump
as follows:
$ psql <<EOF
create table t (i int);
alter table t add constraint t_i_not_null not null i not valid;
comment on constraint t_i_not_null ON t IS 'iii';
EOF
$ pg_dump
Segmentation fault: 11
I'm bothered by this not having any tests -- I'll see about adding some
after lunch.
+1. Thanks!
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On Wed, Jun 25, 2025 at 11:04 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
This commit corrects the behavior by ensuring CREATE TABLE LIKE to also copy
the comments on NOT NULL constraints when INCLUDING COMMENTS is specified.LGTM. I'd add a line in the test showing that these comments are copied
even if INCLUDING CONSTRAINTS is not given,+1 to adding that test.
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1); \d+ ctlt1_inh -SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass; +SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";However, since ctlt1_inh is created with INCLUDING COMMENTS, this test
doesn't seem to demonstrate the case you mentioned — that comments on
not-null constraints are copied even without INCLUDING CONSTRAINTS.
Am I misunderstanding?
to do that,
we can create another table in create_table_like.sql:
CREATE TABLE noinh_con_copy2 (LIKE noinh_con_copy INCLUDING COMMENTS);
or change noinh_con_copy1 definition to
CREATE TABLE noinh_con_copy1 (LIKE noinh_con_copy INCLUDING COMMENTS);
On 2025-Jun-26, Fujii Masao wrote:
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1); \d+ ctlt1_inh -SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass; +SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";However, since ctlt1_inh is created with INCLUDING COMMENTS, this test
doesn't seem to demonstrate the case you mentioned — that comments on
not-null constraints are copied even without INCLUDING CONSTRAINTS.
Am I misunderstanding?
Hmm, yeah the case I wanted to modify was for ctlt12_comments which does
INCLUDING COMMENTS without constraints, apologies. In that case it's
better to modify ctlt2 instead, as shown here.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Attachments:
0001-test-fixup-v2.patch.txttext/plain; charset=utf-8Download
From 7c395756e42d718d3a2a467a33239446d2ca1d02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 25 Jun 2025 17:35:03 +0200
Subject: [PATCH] test fixup v2
---
src/test/regress/expected/create_table_like.out | 15 ++++++++++++---
src/test/regress/sql/create_table_like.sql | 4 +++-
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 1374a972e6e..29a779c2e90 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -332,9 +332,10 @@ COMMENT ON CONSTRAINT ctlt1_a_check ON ctlt1 IS 't1_a_check';
COMMENT ON INDEX ctlt1_pkey IS 'index pkey';
COMMENT ON INDEX ctlt1_b_key IS 'index b_key';
ALTER TABLE ctlt1 ALTER COLUMN a SET STORAGE MAIN;
-CREATE TABLE ctlt2 (c text);
+CREATE TABLE ctlt2 (c text NOT NULL);
ALTER TABLE ctlt2 ALTER COLUMN c SET STORAGE EXTERNAL;
COMMENT ON COLUMN ctlt2.c IS 'C';
+COMMENT ON CONSTRAINT ctlt2_c_not_null ON ctlt2 IS 't2_c_not_null';
CREATE TABLE ctlt3 (a text CHECK (length(a) < 5), c text CHECK (length(c) < 7));
ALTER TABLE ctlt3 ALTER COLUMN c SET STORAGE EXTERNAL;
ALTER TABLE ctlt3 ALTER COLUMN a SET STORAGE MAIN;
@@ -351,9 +352,10 @@ CREATE TABLE ctlt12_storage (LIKE ctlt1 INCLUDING STORAGE, LIKE ctlt2 INCLUDING
--------+------+-----------+----------+---------+----------+--------------+-------------
a | text | | not null | | main | |
b | text | | | | extended | |
- c | text | | | | external | |
+ c | text | | not null | | external | |
Not-null constraints:
"ctlt1_a_not_null" NOT NULL "a"
+ "ctlt2_c_not_null" NOT NULL "c"
CREATE TABLE ctlt12_comments (LIKE ctlt1 INCLUDING COMMENTS, LIKE ctlt2 INCLUDING COMMENTS);
\d+ ctlt12_comments
@@ -362,9 +364,16 @@ CREATE TABLE ctlt12_comments (LIKE ctlt1 INCLUDING COMMENTS, LIKE ctlt2 INCLUDIN
--------+------+-----------+----------+---------+----------+--------------+-------------
a | text | | not null | | extended | | A
b | text | | | | extended | | B
- c | text | | | | extended | | C
+ c | text | | not null | | extended | | C
Not-null constraints:
"ctlt1_a_not_null" NOT NULL "a"
+ "ctlt2_c_not_null" NOT NULL "c"
+
+SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt12_comments'::regclass;
+ conname | description
+------------------+---------------
+ ctlt2_c_not_null | t2_c_not_null
+(1 row)
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1);
NOTICE: merging column "a" with inherited definition
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sql
index 6da7f4f0557..bf8702116a7 100644
--- a/src/test/regress/sql/create_table_like.sql
+++ b/src/test/regress/sql/create_table_like.sql
@@ -143,9 +143,10 @@ COMMENT ON INDEX ctlt1_pkey IS 'index pkey';
COMMENT ON INDEX ctlt1_b_key IS 'index b_key';
ALTER TABLE ctlt1 ALTER COLUMN a SET STORAGE MAIN;
-CREATE TABLE ctlt2 (c text);
+CREATE TABLE ctlt2 (c text NOT NULL);
ALTER TABLE ctlt2 ALTER COLUMN c SET STORAGE EXTERNAL;
COMMENT ON COLUMN ctlt2.c IS 'C';
+COMMENT ON CONSTRAINT ctlt2_c_not_null ON ctlt2 IS 't2_c_not_null';
CREATE TABLE ctlt3 (a text CHECK (length(a) < 5), c text CHECK (length(c) < 7));
ALTER TABLE ctlt3 ALTER COLUMN c SET STORAGE EXTERNAL;
@@ -162,6 +163,7 @@ CREATE TABLE ctlt12_storage (LIKE ctlt1 INCLUDING STORAGE, LIKE ctlt2 INCLUDING
\d+ ctlt12_storage
CREATE TABLE ctlt12_comments (LIKE ctlt1 INCLUDING COMMENTS, LIKE ctlt2 INCLUDING COMMENTS);
\d+ ctlt12_comments
+SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt12_comments'::regclass;
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1);
\d+ ctlt1_inh
SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass;
--
2.39.5
On 2025-Jun-26, Fujii Masao wrote:
However, with the patch applied, I encountered a segmentation fault in pg_dump
as follows:
Ah, thanks for the test case. Yeah, I removed one 'if' condition too
many from Jian's patch. We just need to test the constraint name for
nullness and then things seem to work:
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 5dd1f42631d..7b5024d4f71 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -17719,7 +17719,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) for (j = 0; j < tbinfo->numatts; j++)
{
- if (tbinfo->notnull_comment[j] != NULL)
+ if (tbinfo->notnull_constrs[j] != NULL &&
+ tbinfo->notnull_comment[j] != NULL)
{
if (comment == NULL)
{--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
On 2025-Jun-25, Álvaro Herrera wrote:
Ah, thanks for the test case. Yeah, I removed one 'if' condition too
many from Jian's patch. We just need to test the constraint name for
nullness and then things seem to work:
One more thing was missing, which I noticed as I added the tests.
Apparently the COMMENT objects can end up in any section of the dump,
but for comments on valid constraints, this is not what we want -- they
should go into the PRE_DATA section only. When running the tests with
the code still putting them in SECTION_NONE, apparently pg_dump would
randomly put them either here or there, so the tests would fail
intermittently. Or at least that's what I think happened. Once I made
them be in SECTION_PRE_DATA, that problem disappeared.
Anyway, here's what I intend to push tomorrow, unless further problems
are found.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)
Attachments:
v4-0001-pg_dump-include-comments-on-valid-not-null-constr.patchtext/x-diff; charset=utf-8Download
From 7a4cc26d12da930049a6810b77b35b38deb80e37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 25 Jun 2025 20:18:31 +0200
Subject: [PATCH v4] pg_dump: include comments on valid not-null constraints,
too
We were missing collecting comments for not-null constraints that are
dumped inline with the table definition, because they aren't represented
by a separately dumpable object. Fix by creating separate TocEntries
for them.
Author: Jian He <jian.universality@gmail.com>
Reported-By: Fujii Masao <masao.fujii@oss.nttdata.com>
Discussion: https://postgr.es/m/d50ff977-c728-4e9e-8488-fc2688e08754@oss.nttdata.com
---
src/bin/pg_dump/pg_dump.c | 97 +++++++++++++++++++++++++++++---
src/bin/pg_dump/pg_dump.h | 1 +
src/bin/pg_dump/t/002_pg_dump.pl | 32 ++++++++++-
3 files changed, 120 insertions(+), 10 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index db944ec2230..99969428f9d 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,65 @@ 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_constrs[j] != NULL &&
+ 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);
+
+ /*
+ * These entries exist only for (valid) constraints that are
+ * dumped together with the table definition, so they're all
+ * in SECTION_PRE_DATA.
+ */
+ ArchiveEntry(fout, nilCatalogId, createDumpId(),
+ ARCHIVE_OPTS(.tag = tag->data,
+ .namespace = tbinfo->dobj.namespace->dobj.name,
+ .owner = owner,
+ .description = "COMMENT",
+ .section = SECTION_PRE_DATA,
+ .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 */
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 386e21e0c59..e1cfa99874e 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1191,7 +1191,9 @@ my %tests = (
) INHERITS (dump_test.test_table_nn, dump_test.test_table_nn_2);
ALTER TABLE dump_test.test_table_nn ADD CONSTRAINT nn NOT NULL col1 NOT VALID;
ALTER TABLE dump_test.test_table_nn_chld1 VALIDATE CONSTRAINT nn;
- ALTER TABLE dump_test.test_table_nn_chld2 VALIDATE CONSTRAINT nn;',
+ ALTER TABLE dump_test.test_table_nn_chld2 VALIDATE CONSTRAINT nn;
+ COMMENT ON CONSTRAINT nn ON dump_test.test_table_nn IS \'nn comment is valid\';
+ COMMENT ON CONSTRAINT nn ON dump_test.test_table_nn_chld2 IS \'nn_chld2 comment is valid\';',
regexp => qr/^
\QALTER TABLE dump_test.test_table_nn\E \n^\s+
\QADD CONSTRAINT nn NOT NULL col1 NOT VALID;\E
@@ -1205,6 +1207,34 @@ my %tests = (
},
},
+ # This constraint is invalid therefore it goes in SECTION_POST_DATA
+ 'COMMENT ON CONSTRAINT ON test_table_nn' => {
+ regexp => qr/^
+ \QCOMMENT ON CONSTRAINT nn ON dump_test.test_table_nn IS\E
+ /xm,
+ like => {
+ %full_runs, %dump_test_schema_runs, section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ # This constraint is valid therefore it goes in SECTION_PRE_DATA
+ 'COMMENT ON CONSTRAINT ON test_table_chld2' => {
+ regexp => qr/^
+ \QCOMMENT ON CONSTRAINT nn ON dump_test.test_table_nn_chld2 IS\E
+ /xm,
+ like => {
+ %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
'CONSTRAINT NOT NULL / NOT VALID (child1)' => {
regexp => qr/^
\QCREATE TABLE dump_test.test_table_nn_chld1 (\E\n
--
2.39.5
On 2025/06/26 3:45, Álvaro Herrera wrote:
On 2025-Jun-25, Álvaro Herrera wrote:
Ah, thanks for the test case. Yeah, I removed one 'if' condition too
many from Jian's patch. We just need to test the constraint name for
nullness and then things seem to work:One more thing was missing, which I noticed as I added the tests.
Apparently the COMMENT objects can end up in any section of the dump,
but for comments on valid constraints, this is not what we want -- they
should go into the PRE_DATA section only. When running the tests with
the code still putting them in SECTION_NONE, apparently pg_dump would
randomly put them either here or there, so the tests would fail
intermittently. Or at least that's what I think happened. Once I made
them be in SECTION_PRE_DATA, that problem disappeared.Anyway, here's what I intend to push tomorrow, unless further problems
are found.
Thanks for updating the patch!
I noticed a small inconsistency in the output of pg_dump when handling
comments for COMMENT commands on not-null constraints. For example,
with the following DDL:
-------------
CREATE SCHEMA hoge;
CREATE TABLE hoge.t (i int PRIMARY KEY, j int NOT NULL CHECK (j > 0));
COMMENT ON CONSTRAINT t_pkey ON hoge.t IS 'primary key';
COMMENT ON CONSTRAINT t_j_not_null ON hoge.t IS 'not null';
COMMENT ON CONSTRAINT t_j_check ON hoge.t IS 'check';
-------------
The pg_dump output shows the following comments for COMMENT commands:
-------------
-- Name: CONSTRAINT t_j_not_null ON hoge.t; Type: COMMENT; Schema: hoge; Owner: postgres
-- Name: CONSTRAINT t_j_check ON t; Type: COMMENT; Schema: hoge; Owner: postgres
-- Name: CONSTRAINT t_pkey ON t; Type: COMMENT; Schema: hoge; Owner: postgres
-------------
You can see that only comments for the not-null constraint includes
the schema-qualified table name (hoge.t) after "ON". The others just
show "t". It's a very minor issue, but for consistency, it would be
better if all constraint comments used the same format.
+ if (comment != NULL)
+ {
+ destroyPQExpBuffer(comment);
+ destroyPQExpBuffer(tag);The "comment != NULL" check isn't needed here, since destroyPQExpBuffer()
already handles null safely.
Since valid not-null constraints are dumped in the pre-data section,
the following change seems necessary in pg_dump.sgml.
statistics for indexes, and constraints other than validated check
- constraints.
+ and not-null constraints.
Pre-data items include all other data definition items.Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025/06/26 0:45, Álvaro Herrera wrote:
On 2025-Jun-26, Fujii Masao wrote:
CREATE TABLE ctlt1_inh (LIKE ctlt1 INCLUDING CONSTRAINTS INCLUDING COMMENTS) INHERITS (ctlt1); \d+ ctlt1_inh -SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass; +SELECT conname, description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt1_inh'::regclass ORDER BY conname COLLATE "C";However, since ctlt1_inh is created with INCLUDING COMMENTS, this test
doesn't seem to demonstrate the case you mentioned — that comments on
not-null constraints are copied even without INCLUDING CONSTRAINTS.
Am I misunderstanding?Hmm, yeah the case I wanted to modify was for ctlt12_comments which does
INCLUDING COMMENTS without constraints, apologies. In that case it's
better to modify ctlt2 instead, as shown here.
Thanks for updating the tests!
I've incorporated the changes into the patch and committed it.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
On 2025-Jun-26, Fujii Masao wrote:
I noticed a small inconsistency in the output of pg_dump when handling
comments for COMMENT commands on not-null constraints. [...]
You can see that only comments for the not-null constraint includes
the schema-qualified table name (hoge.t) after "ON". The others just
show "t". It's a very minor issue, but for consistency, it would be
better if all constraint comments used the same format.
Hmm, you're right. I think we should make the tags use qualified names
instead, but that'd be a much larger patch.
+ if (comment != NULL) + { + destroyPQExpBuffer(comment); + destroyPQExpBuffer(tag);The "comment != NULL" check isn't needed here, since destroyPQExpBuffer()
already handles null safely.
Hah, true.
Since valid not-null constraints are dumped in the pre-data section,
the following change seems necessary in pg_dump.sgml.statistics for indexes, and constraints other than validated check - constraints. + and not-null constraints. Pre-data items include all other data definition items.
Ah, right, did that, thank you.
Thanks for reporting this and to Jian for the quick analysis and patch!
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Oh, great altar of passive entertainment, bestow upon me thy discordant images
at such speed as to render linear thought impossible" (Calvin a la TV)