Date: Fri, 12 Sep 2025 19:02:33 -0700
From: Noah Misch <noah@leadboat.com>
To: =?iso-8859-1?Q?=C1lvaro?= Herrera <alvherre@kurilemu.de>
Cc: jian he <jian.universality@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump does not dump domain not-null constraint's comments
Message-ID: <20250913020233.fa.nmisch@google.com>
References: 
 <CACJufxHg-KABN2FyKgY4bBM=pa0ijpwarKyCA0F+0Dqb_TjjCA@mail.gmail.com>
 <202507151340.d4jyrbdae5af@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202507151340.d4jyrbdae5af@alvherre.pgsql>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20250913020233.fa.nmisch%40google.com>
Precedence: bulk

On Tue, Jul 15, 2025 at 03:40:38PM +0200, Álvaro Herrera wrote:
> On 2025-Jul-15, jian he wrote:
> > 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.

This became commit 0858f0f.

> @@ -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);

The last argument gives the dump object on which the comment has a dependency.
Since this is the case of a separately-dumped constraint, the comment needs to
depend on that constraint (coninfo), not on the domain (tyinfo):

-							coninfo->dobj.catId, 0, tyinfo->dobj.dumpId);
+							coninfo->dobj.catId, 0, coninfo->dobj.dumpId);

I didn't encounter a failure from this, but sufficient restore parallelism
might reach a failure.  A failure would look like a "does not exist" error in
the COMMENT command, due to the constraint not yet existing.
dumpTableConstraintComment() is an older case that optimally handles the last
dumpComment() arg.

In the absence of objections, I'll make it so.