Feedback-ID: ie3de48e3:Fastmail
Date: Tue, 15 Jul 2025 15:40:38 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: jian he <jian.universality@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump does not dump domain not-null constraint's comments
Message-ID: <202507151340.d4jyrbdae5af@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="2pg2gr6gj4qxbkrc"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CACJufxHg-KABN2FyKgY4bBM=pa0ijpwarKyCA0F+0Dqb_TjjCA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202507151340.d4jyrbdae5af%40alvherre.pgsql>
Precedence: bulk


--2pg2gr6gj4qxbkrc
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Jul-15, jian he wrote:

> On Tue, Jul 15, 2025 at 2:10 PM jian he <jian.universality@gmail.com> wrote:

> > accidently found another existing bug.

> > CREATE SCHEMA test;
> > CREATE DOMAIN test.d1 AS integer NOT NULL DEFAULT 11;
> > COMMENT ON CONSTRAINT a2 ON DOMAIN test.d1 IS 'test';
> > ALTER DOMAIN test.d1
> >     ADD CONSTRAINT a2 CHECK ((VALUE > 1)) NOT VALID;
> >
> > Obviously the COMMENT command will error out.
> > currently working on a fix, just sharing the bug details in advance.

Ouch, ugh.  I think this is as old as NOT VALID constraints on domains,
maybe from commit 897795240cfa (Jun 2011), or ... no, actually
7eca575d1c28 (Dec 2014) which enabled constraints on domains to have
comments.  In either case it's my fault, and the fix needs to be
backpatched all the way back, so I'm going to apply this bugfix one
ahead of the others, which are for newer bugs.

> we should let:
> dumpConstraint handles dumping separate "NOT VALID" domain constraints along
> with their comments.
> dumpDomain: handles dumping "inlined" valid (not separate) domain constraints
> together with their comments.

Yeah, this makes sense.

> tested locally, i didn't write the test on src/bin/pg_dump/t/002_pg_dump.pl....

I'll add something for coverage, but not yet sure if it's going to be
something in 002_pg_dump.pl, or objects to be left around for the
pg_upgrade test to pick up.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

--2pg2gr6gj4qxbkrc
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Fix-dumping-of-comments-on-invalid-constraints-on-do.patch"

From fa407dccc9782d8f3accdcf4045ab9bb8859aef9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Tue, 15 Jul 2025 15:24:33 +0200
Subject: [PATCH] Fix dumping of comments on invalid constraints on domains

We skip dumping constraints together with domains if they are invalid
('separate') so that they appear after data -- but their comments were
dumped together with the domain definition, which in effect leads to the
comment being dumped when the constraint does not yet exist.  Delay
them in the same way.

Oversight in 7eca575d1c28; backpatch all the way back.

Author: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxF_C2pe6J_+nPr6C5jf5rQnbYP8XOKr4HM8yHZtp2aQqQ@mail.gmail.com
---
 src/bin/pg_dump/pg_dump.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1937997ea67..5af2ac151c2 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -12582,9 +12582,15 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
 	/* Dump any per-constraint comments */
 	for (i = 0; i < tyinfo->nDomChecks; i++)
 	{
 		ConstraintInfo *domcheck = &(tyinfo->domChecks[i]);
-		PQExpBuffer conprefix = createPQExpBuffer();
+		PQExpBuffer conprefix;
+
+		/* but only if the constraint itself was dumped here */
+		if (domcheck->separate)
+			continue;
+
+		conprefix = createPQExpBuffer();
 
 		appendPQExpBuffer(conprefix, "CONSTRAINT %s ON DOMAIN",
 						  fmtId(domcheck->dobj.name));
 
@@ -18487,8 +18493,25 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
 										  .description = "CHECK CONSTRAINT",
 										  .section = SECTION_POST_DATA,
 										  .createStmt = q->data,
 										  .dropStmt = delq->data));
+
+			if (coninfo->dobj.dump & DUMP_COMPONENT_COMMENT)
+			{
+				char	   *qtypname;
+
+				PQExpBuffer conprefix = createPQExpBuffer();
+				qtypname = pg_strdup(fmtId(tyinfo->dobj.name));
+
+				appendPQExpBuffer(conprefix, "CONSTRAINT %s ON DOMAIN",
+								  fmtId(coninfo->dobj.name));
+
+				dumpComment(fout, conprefix->data, qtypname,
+							tyinfo->dobj.namespace->dobj.name,
+							tyinfo->rolname,
+							coninfo->dobj.catId, 0, tyinfo->dobj.dumpId);
+				destroyPQExpBuffer(conprefix);
+			}
 		}
 	}
 	else
 	{
-- 
2.39.5


--2pg2gr6gj4qxbkrc--