Re: not null constraints, again

On Thu, Sep 19, 2024 at 4:26 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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)