Re: not null constraints, again

On Wed, Sep 11, 2024 at 2:18 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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?