Thread: Re: Support for NO INHERIT to INHERIT state change with named NOT NULL constraints

On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <suraj.kharage@enterprisedb.com> wrote:
Alvaro stated that allowing a not null constraint state to be modified from INHERIT to NO INHERIT is going to be quite problematic because of the number of weird cases to avoid, so for now that support is not added.

What's the reasoning behind that restriction? What are the weird cases? 

--
On Fri, Nov 15, 2024 at 11:15 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <suraj.kharage@enterprisedb.com> wrote:
>>
>> Alvaro stated that allowing a not null constraint state to be modified from INHERIT to NO INHERIT is going to be
quiteproblematic because of the number of weird cases to avoid, so for now that support is not added. 
>
> What's the reasoning behind that restriction? What are the weird cases?

current status:
drop table if exists idxpart,idxpart0,idxpart1 cascade;
create table idxpart (a int not null) partition by list (a);
create table idxpart0 (a int constraint foo not null no inherit);

alter table idxpart attach partition idxpart0 for values in (0,1);
ERROR:  constraint "foo" conflicts with non-inherited constraint on
child table "idxpart0"

to make it attach to the partition, we need to drop and recreate the
not-null constraint "foo".
that would be very expensive, since recreate, we need to revalidate
the previous row is not null or not.
related post:
https://www.postgresql.org/message-id/202410021219.bvjmxzdspif2%40alvherre.pgsql

with
alter table idxpart0 alter constraint foo inherit;

then we can

alter table idxpart attach partition idxpart0 for values in (0,1);




--

Thanks & Regards, 
Suraj kharage, 



On Tue, Nov 19, 2024 at 6:52 PM jian he <jian.universality@gmail.com> wrote:
On Fri, Nov 15, 2024 at 11:15 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Nov 14, 2024 at 12:02 AM Suraj Kharage <suraj.kharage@enterprisedb.com> wrote:
>>
>> Alvaro stated that allowing a not null constraint state to be modified from INHERIT to NO INHERIT is going to be quite problematic because of the number of weird cases to avoid, so for now that support is not added.
>
> What's the reasoning behind that restriction? What are the weird cases?

current status:
drop table if exists idxpart,idxpart0,idxpart1 cascade;
create table idxpart (a int not null) partition by list (a);
create table idxpart0 (a int constraint foo not null no inherit);

alter table idxpart attach partition idxpart0 for values in (0,1);
ERROR:  constraint "foo" conflicts with non-inherited constraint on
child table "idxpart0"

to make it attach to the partition, we need to drop and recreate the
not-null constraint "foo".
that would be very expensive, since recreate, we need to revalidate
the previous row is not null or not.
related post:
https://www.postgresql.org/message-id/202410021219.bvjmxzdspif2%40alvherre.pgsql

Right.
Another case which needs conclusion is -
When changing from INHERIT to NO INHERIT, we need to walk all children and decrement coninhcount for the corresponding constraint. If a constraint in one child reaches zero, should we drop it?  not sure. If we do, make sure to reset the corresponding attnotnull bit too. We could decide not to drop the constraint, in which case you don’t need to reset attnotnull.
 


with
alter table idxpart0 alter constraint foo inherit;

then we can

alter table idxpart attach partition idxpart0 for values in (0,1);
On Tue, Nov 19, 2024 at 8:22 AM jian he <jian.universality@gmail.com> wrote:
> current status:
> drop table if exists idxpart,idxpart0,idxpart1 cascade;
> create table idxpart (a int not null) partition by list (a);
> create table idxpart0 (a int constraint foo not null no inherit);
>
> alter table idxpart attach partition idxpart0 for values in (0,1);
> ERROR:  constraint "foo" conflicts with non-inherited constraint on
> child table "idxpart0"
>
> to make it attach to the partition, we need to drop and recreate the
> not-null constraint "foo".
> that would be very expensive, since recreate, we need to revalidate
> the previous row is not null or not.

In a simple implementation of ALTER TABLE this would be true, but I
don't see why it should need to be true in ours. It should be possible
to notice that there's an existing NOT NULL constraint and use that as
evidence that the new one can be added without needing to revalidate
the table contents. ALTER TABLE does similar things already. For
instance, TryReuseIndex() can attempt to attach an existing index file
to a new index definition without rebuilding it; TryReuseForeignKey
can attempt to re-add a foreign key constraint without needing to
revalidate it. But even more to the point, ATAddCheckNNConstraint and
MergeWithExistingConstraint know about merging a newly-added
constraint with a preexisting one without needing to revalidate the
table.

--
Robert Haas
EDB: http://www.enterprisedb.com