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

On Tue, Oct 1, 2024 at 11:20 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Oct-01, jian he wrote:
>
> > create table t2 (a int primary key  constraint foo not null no inherit);
> > primary key cannot coexist with not-null no inherit?
> > here t2, pg_dump/restore will fail.
>
> Yeah, this needs to throw an error.  If you use a table constraint, it
> does fail as expected:
>
> create table notnull_tbl_fail (a int primary key, not null a no inherit);
> ERROR:  conflicting NO INHERIT declaration for not-null constraint on column "a"
>
> I missed adding the check in the column constraint case.
>
after v7, still not bullet-proof. as before, pg_dump/restore will fail
for the following:

drop table if exists t2, t2_0
create table t2 (a int, b int, c int, constraint foo primary key(a),
constraint foo1 not null a no inherit);
create table t2_0 (a int constraint foo1 not null no inherit, b int, c
int, constraint foo12 primary key(a));



> > +      By contrast, a <literal>NOT NULL</literal> constraint that was created
> > +      as <literal>NO INHERIT</literal> will be changed to a normal inheriting
> > +      one during attach.
> > Does this sentence don't have corresponding tests?
> > i think you mean something like:
> >
> > 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,NULL);
>
> I think we could just remove this behavior and nothing of value would be
> lost.  If I recall correctly, handling of NO INHERIT constraints in this
> way was just added to support the old way of adding PRIMARY KEY, but it
> feels like a wart that's easily fixed and not worth having, because it's
> just weird.  I mean, what's the motivation for having created the
> partition (resp. child table) with a NO INHERIT constraint in the first
> place?
>
>
with your v7 change, you need remove:
> > +      By contrast, a <literal>NOT NULL</literal> constraint that was created
> > +      as <literal>NO INHERIT</literal> will be changed to a normal inheriting
> > +      one during attach.


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);

With V7, we basically cannot change the status of "NO INHERIT".
now, we need to drop the not-null constraint foo,
recreate a not-null constraint on idxpart0,
then attach it to the partitioned table idxpart.

imagine a scenario where:
At first we didn't know that the NO INHERIT not-null constraint would
be attached to a partitioned table.
If we want, then we hope attaching it to a partitioned table would be easier.
As you can see, v7 will make idxpart0 attach to idxpart quite difficult.


---------------------------------------------------------------------------------
drop table if exists inh_parent,inh_child1,inh_child2;
create table inh_parent(f1 int);
create table inh_child1(f1 int not null);
alter table inh_child1 inherit inh_parent;
alter table only inh_parent add constraint nn not null f1;
alter table only inh_parent  alter column f1 set not null;

minor inconsistency, i guess.
"alter table only inh_parent add constraint nn not null f1;"
will fail.
But
"alter table only inh_parent  alter column f1 set not null;"
will not fail, but add a "NOT NULL f1 NO INHERIT" constraint.
I thought they should behave the same.


for partitioned table
now both ALTER TABLE ONLY ADD CONSTRAINT NOT NULL,
ALTER TABLE ONLY ALTER COLUMN  SET NOT NULL
will error out.
I am fine with partitioned table behavior.