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

On Wed, Sep 11, 2024 at 2:18 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Hello, here's a v2 of this patch.  I have fixed --I think-- all the
> issues you and Tender Wang reported (unless I declined a fix in some
> previous email).
>

+ /*
+ * The constraint must appear as inherited in children, so create a
+ * modified constraint object to use.
+ */
+ constr = copyObject(constr);
+ constr->inhcount = 1;

in ATAddCheckNNConstraint, we don't need the above copyObject call.
because at the beginning of ATAddCheckNNConstraint, we do
    newcons = AddRelationNewConstraints(rel, NIL,
                                        list_make1(copyObject(constr)),
                                        recursing || is_readd,    /*
allow_merge */
                                        !recursing, /* is_local */
                                        is_readd,    /* is_internal */
                                        NULL);    /* queryString not available
                                                 * here */


pg_constraint manual <<<<QUOTE<<<
conislocal bool
This constraint is defined locally for the relation. Note that a
constraint can be locally defined and inherited simultaneously.
coninhcount int2
The number of direct inheritance ancestors this constraint has. A
constraint with a nonzero number of ancestors cannot be dropped nor
renamed.
<<<<END OF QUOTE

drop table idxpart cascade;
create table idxpart (a int) partition by range (a);
create table idxpart0 (like idxpart);
alter table idxpart0 add primary key (a);
alter table idxpart attach partition idxpart0 for values from (0) to (1000);
alter table idxpart add primary key (a);

alter table idxpart0 DROP CONSTRAINT idxpart0_pkey;
alter table idxpart0 DROP CONSTRAINT idxpart0_a_not_null;

First DROP CONSTRAINT failed as the doc said,
but the second success.
but the second DROP CONSTRAINT should fail?
Even if you drop success, idxpart0_a_not_null still exists.
it also conflicts with the pg_constraint I've quoted above.


transformTableLikeClause, expandTableLikeClause
can be further simplified when the relation don't have not-null as all like:

    /*
     * Reproduce not-null constraints by copying them.  This doesn't require
     * any option to have been given.
     */
    if (tupleDesc->constr && tupleDesc->constr->has_not_null)
    {
        lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false);
        cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
    }




we can do:
create table parent (a text, b int);
create table child () inherits (parent);
alter table child no inherit parent;

so comments in AdjustNotNullInheritance
 * AdjustNotNullInheritance
 *        Adjust not-null constraints' inhcount/islocal for
 *        ALTER TABLE [NO] INHERITS

"ALTER TABLE [NO] INHERITS"
should be
"ALTER TABLE ALTER COLUMN [NO] INHERITS"
?

Also, seems AdjustNotNullInheritance never being called/used?