Thread: Re: not null constraints, again

On Fri, Oct 4, 2024 at 9:11 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Here's v8 of this patch.


in AdjustNotNullInheritance
        if (count > 0)
        {
            conform->coninhcount += count;
            changed = true;
        }
        if (is_local)
        {
            conform->conislocal = true;
            changed = true;
        }

change to

        if (count > 0)
        {
            conform->coninhcount += count;
            changed = true;
        }
        if (is_local && !conform->conislocal)
        {
            conform->conislocal = true;
            changed = true;
        }

then we can save some cycles.

-------------------<<>>>>------------
MergeConstraintsIntoExisting
            /*
             * If the CHECK child constraint is "no inherit" then cannot
             * merge.
             */
            if (child_con->connoinherit)
                ereport(ERROR,
                        (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                         errmsg("constraint \"%s\" conflicts with
non-inherited constraint on child table \"%s\"",
                                NameStr(child_con->conname),
RelationGetRelationName(child_rel))));
the comments apply to not-null constraint aslo, so the comments need
to be refactored.

-------------------<<>>>>------------
in ATExecSetNotNull
        if (recursing)
        {
            conForm->coninhcount++;
            changed = true;
        }

grep "coninhcount++", I found out pattern:
        constrForm->coninhcount++;
        if (constrForm->coninhcount < 0)
            ereport(ERROR,
                    errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                    errmsg("too many inheritance parents"));

here, maybe we can change to
        if (recursing)
        {
            // conForm->coninhcount++;
            if (pg_add_s16_overflow(conForm->coninhcount,1,
&conForm->coninhcount))
                ereport(ERROR,
                        errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
                        errmsg("too many inheritance parents"));
            changed = true;
        }
-------------------<<>>>>------------
base on your reply at [1]

      By contrast, a <literal>NOT NULL</literal> constraint that was created
      as <literal>NO INHERIT</literal> will be changed to a normal inheriting
      one during attach.

these text should removed from section:
<<ATTACH PARTITION partition_name { FOR VALUES partition_bound_spec |
DEFAULT }>>
since currently v8, partition_name not-null no inherit constraint
cannot merge with the parent.

[1] https://www.postgresql.org/message-id/202410021219.bvjmxzdspif2%40alvherre.pgsql

tricky case:
drop table if exists part, part0 cascade;
create table part (a int not null) partition by range (a);
create table part0 (a int primary key);
alter table part attach partition part0 for values from (0) to (1000);
alter table ONLY part add primary key(a);
alter table ONLY part drop constraint part_a_not_null;
-- alter table ONLY part alter column a drop not null;


Now we are in a state where a partitioned
table have a primary key but doesn't have a not-null constraint for it.

select  indisunique, indisprimary, indimmediate,indisvalid
from    pg_index
where   indexrelid = 'part_pkey'::regclass;

shows this primary key index is invalid.

but
select conname,contype,convalidated
from pg_constraint where conname = 'part_pkey';

shows this primary key constraint is valid.