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

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