Re: not null constraints, again

[jian he]

On Thu, Sep 19, 2024 at 4:26 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
>
>
> > drop table if exists idxpart, idxpart0, idxpart1 cascade;
> > create table idxpart (a int) partition by range (a);
> > create table idxpart0 (a int primary key);
> > alter table idxpart attach partition idxpart0 for values from (0) to (1000);
> > alter table idxpart alter column a set not null;
> > alter table idxpart0 alter column a drop not null;
> > alter table idxpart0 drop constraint idxpart0_a_not_null;
> >
> > "alter table idxpart0 alter column a drop not null;"
> > is logically equivalent to
> > "alter table idxpart0 drop constraint idxpart0_a_not_null;"
> >
> > the first one (alter column) ERROR out,
> > the second success.
> > the second "drop constraint" should also ERROR out?
> > since it violates the sentence in ddl-partitioning.html
> > "You cannot drop a NOT NULL constraint on a partition's column if the
> > same constraint is present in the parent table."
>
> Yeah, I modified this code already a few days ago, and now it does error
> out like this
>
> ERROR:  cannot drop inherited constraint "idxpart0_a_not_null" of relation 
> "idxpart0"
>
> Anyway, as I mentioned back then, the DROP CONSTRAINT didn't _actually_
> remove the constraint; it only marked the constraint as no longer
> locally defined (conislocal=false), which had no practical effect other
> than changing the representation during pg_dump.  Even detaching the
> partition after having "dropped" the constraint would make the not-null
> constraint appear again as coninhcount=0,conislocal=true rather than
> drop it.
>

funny.
as the previously sql example, if you execute
"alter table idxpart0 drop constraint idxpart0_a_not_null;"
again

then
ERROR:  cannot drop inherited constraint "idxpart0_a_not_null" of
relation "idxpart0"

I am not sure if that's logically OK or if the user can deduce the
logic from the manual.
like, the first time you use "alter table drop constraint"
to drop a constraint, the constraint is not totally dropped,
the second time you execute it again the constraint cannot be dropped directly.


i think the issue is the changes we did in dropconstraint_internal
in dropconstraint_internal, we have:
-----------
    if (con->contype == CONSTRAINT_NOTNULL &&
        con->conislocal && con->coninhcount > 0)
    {
        HeapTuple    copytup;
        copytup = heap_copytuple(constraintTup);
        con = (Form_pg_constraint) GETSTRUCT(copytup);
        con->conislocal = false;
        CatalogTupleUpdate(conrel, &copytup->t_self, copytup);
        ObjectAddressSet(conobj, ConstraintRelationId, con->oid);
        CommandCounterIncrement();
        table_close(conrel, RowExclusiveLock);
        return conobj;
    }
    /* Don't allow drop of inherited constraints */
    if (con->coninhcount > 0 && !recursing)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                 errmsg("cannot drop inherited constraint \"%s\" of
relation \"%s\"",
                        constrName, RelationGetRelationName(rel))));
-----------



comments in dropconstraint_internal
"* Reset pg_constraint.attnotnull, if this is a not-null constraint."
should be
"pg_attribute.attnotnull"



also, we don't have tests for not-null constraint similar to check
constraint tests on
src/test/regress/sql/alter_table.sql (line 2067 to line 2073)