Re: not null constraints, again - Mailing list pgsql-hackers

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