Feedback-ID: ia2694551:Fastmail
Date: Fri, 28 Mar 2025 19:42:22 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: jian he <jian.universality@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Rushabh Lathia <rushabh.lathia@gmail.com>,
 Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Support NOT VALID / VALIDATE constraint options for named NOT
 NULL constraints
Message-ID: <202503281842.iiuztzg6ksxi@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CACJufxGnXTj59WM_qqH_JNQ2xC8HQNbJdhAiXnCS2vr3j_17GA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202503281842.iiuztzg6ksxi%40alvherre.pgsql>
Precedence: bulk

On 2025-Mar-28, jian he wrote:

> i think your patch messed up with pg_constraint.conislocal.
> for example:
> 
> CREATE TABLE parted (id bigint default 1,id_abc bigint) PARTITION BY LIST (id);
> alter TABLE parted add CONSTRAINT dummy_constr not null id not valid;
> CREATE TABLE parted_1 (id bigint default 1,id_abc bigint);
> alter TABLE parted_1 add CONSTRAINT dummy_constr not null id;
> ALTER TABLE parted ATTACH PARTITION parted_1 FOR VALUES IN ('1');
> 
> select conrelid::regclass, conname, conislocal
> from pg_constraint where conname = 'dummy_constr';
> 
>  conrelid |   conname    | conislocal
> ----------+--------------+------------
>  parted   | dummy_constr | t
>  parted_1 | dummy_constr | f
> (2 rows)
> 
> 
> if you  do pg_dump, and execute the pg_dump output
> pg_dump  --no-statistics --clean --table-and-children=*parted*
> --no-owner --verbose --column-inserts --file=dump.sql --no-acl
> 
> select conrelid::regclass, conname, conislocal
> from pg_constraint where conname = 'dummy_constr';
> output is
> 
>  conrelid |   conname    | conislocal
> ----------+--------------+------------
>  parted   | dummy_constr | t
>  parted_1 | dummy_constr | t
> (2 rows)

Interesting.  Yeah, I removed the code you had there because it was
super weird, had no comments, and removing it had zero effect (no tests
failed), so I thought it was useless.  But apparently something is going
on here that's not what we want.

To fix this, we could say that pg_dump should realize the difference and
dump in a different way ... however I think that'd require looking at
conislocal differently, which I definitely don't want to mess with.

Maybe the real problem here is that making the (valid) child constraint
no longer local when the parent constraint is not valid is not sensible,
precisely because pg_dump won't be able to produce good output.  That
sounds more workable to me ... except that we'd have to ensure that
validating the parent constraint would turn the child constraints as not
local anymore, which might be a bit weird.  But maybe not weirder than
the other approach.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"I must say, I am absolutely impressed with what pgsql's implementation of
VALUES allows me to do. It's kind of ridiculous how much "work" goes away in
my code.  Too bad I can't do this at work (Oracle 8/9)."       (Tom Allison)
           http://archives.postgresql.org/pgsql-general/2007-06/msg00016.php