pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Álvaro 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
Date: Mon, 21 Jul 2025 11:48:52 +0200
Message-ID: <202507210948.lljgqgrrnm5v@alvherre.pgsql> (raw)
In-Reply-To: <CACJufxHEX9nboSZSz_Qw1_Kwvnvdfieqn_rCRHcwEqyGEEzNAA@mail.gmail.com>

On 2025-Jul-18, jian he wrote:

> one minor issue in getDomainConstraints:
> 
>  for (int i = 0, j = 0; i < ntups; i++)
> {
>         char        contype = (PQgetvalue(res, i, i_contype))[0];
>         ....
>         constraint->contype = *(PQgetvalue(res, i, i_contype));
> }
> 
> for the same code simplicity,
> ``constraint->contype = *(PQgetvalue(res, i, i_contype));``
> can change to
> `` constraint->contype = contype;  ``
> ?
> other than that, looks good to me.

Thanks, changed it that way.

I noticed some other issues however.  First, you had removed the contype
comparisons in repairDependencyLoop; I put them back several versions of
the patch ago, but I had mistakenly made them reference the wrong array
item -- in all three places.  Doh.

Second, the name comparisons to determine whether to list the constraint
name in the "CREATE DOMAIN .. NOT NULL" syntax was wrong: it was using
fmtId() around the constraint name, but of course that would have never
worked, because we're comparing to the original string, not a quoted
name.  So if you had a domain called, say
  CREATE DOMAIN "my domain" AS int NOT NULL;

then pg_dump would have said
  CREATE DOMAIN "my domain" AS int CONSTRAINT "my domain_not_null" NOT NULL;

even though listing the name is unnecessary.  This wouldn't have made
the dump fail, so it's borderline; but it would be /slightly/ ugly.

Lastly, I made dumpDomain print unadorned "NOT NULL" if a pg_constraint
row for the constraint is not found and yet we observe typnotnull set.
This can be considered catalog corruption (the row should be there), but
I think this is better than having pg_dump just crash if that row is not
there.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
Voy a acabar con todos los humanos / con los humanos yo acabaré
voy a acabar con todos (bis) / con todos los humanos acabaré ¡acabaré! (Bender)





view thread (16+ messages)  latest in thread

Message-ID: <202507210948.lljgqgrrnm5v@alvherre.pgsql>
Permalink:  ../202507210948.lljgqgrrnm5v@alvherre.pgsql/
Also on:    postgresql.org/message-id/202507210948.lljgqgrrnm5v@alvherre.pgsql

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@kurilemu.de, jian.universality@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: pg_dump does not dump domain not-null constraint's comments
  In-Reply-To: <202507210948.lljgqgrrnm5v@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox