Re: Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints

On Sat, Mar 29, 2025 at 2:42 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> 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.
>

my change in AdjustNotNullInheritance is copied from
MergeConstraintsIntoExisting
``
/*
* OK, bump the child constraint's inheritance count. (If we fail
* later on, this change will just roll back.)
*/
child_copy = heap_copytuple(child_tuple);
child_con = (Form_pg_constraint) GETSTRUCT(child_copy);
if (pg_add_s16_overflow(child_con->coninhcount, 1,
&child_con->coninhcount))
ereport(ERROR,
errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("too many inheritance parents"));
/*
* In case of partitions, an inherited constraint must be
* inherited only once since it cannot have multiple parents and
* it is never considered local.
*/
if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
Assert(child_con->coninhcount == 1);
child_con->conislocal = false;
}
``

if you look at MergeConstraintsIntoExisting, then it won't be weird.
AdjustNotNullInheritance is kind of doing the same thing as
MergeConstraintsIntoExisting, I think.