Re: not null constraints, again

On 2024-Sep-19, jian he wrote:

> still based on v3.
> in src/sgml/html/ddl-partitioning.html
> <<<QUOTE<<
> Both CHECK and NOT NULL constraints of a partitioned table are always
> inherited by all its partitions.
> CHECK constraints that are marked NO INHERIT are not allowed to be
> created on partitioned tables.
> You cannot drop a NOT NULL constraint on a partition's column if the
> same constraint is present in the parent table.
> <<<QUOTE<<
> we can change
> "CHECK constraints that are marked NO INHERIT are not allowed to be
> created on partitioned tables."
> to
> "CHECK and NOT NULL constraints that are marked NO INHERIT are not
> allowed to be created on partitioned tables."

Right. Your proposed text is correct but sounds a bit repetitive with
the phrase just prior, and also the next one about inability to drop a
NOT NULL applies equally to CHECK constraints; so I modified the whole
paragraph to this:

Both <literal>CHECK</literal> and <literal>NOT NULL</literal>
constraints of a partitioned table are always inherited by all its
partitions; it is not allowed to create <literal>NO INHERIT<literal>
constraints of those types.
You cannot drop a constraint of those types if the same constraint
is present in the parent table.

> in sql-altertable.html we have:
> <<<QUOTE<<
> ATTACH PARTITION partition_name { FOR VALUES partition_bound_spec | DEFAULT }
> If any of the CHECK constraints of the table being attached are marked
> NO INHERIT, the command will fail; such constraints must be recreated
> without the NO INHERIT clause.
> <<<QUOTE<<
>
> create table idxpart (a int constraint nn not null) partition by range (a);
> create table idxpart0 (a int constraint nn not null no inherit);
> alter table idxpart attach partition idxpart0 for values from (0) to (1000);
>
> In the above sql query case,
> we changed a constraint ("nn" on idxpart0) connoinherit attribute
> after ATTACH PARTITION.
> (connoinherit from true to false)
> Do we need extra sentences to explain it?
> here not-null constraint behavior seems to divert from CHECK constraint.

Ah, yeah, the docs are misleading: we do allow these constraints to
mutate from NO INHERIT to INHERIT. There's no danger in this, because
such a table cannot have children: no inheritance children (because
inheritance-parent tables cannot be partitions) and no partitions
either, because partitioned tables are not allowed to have NOT NULL NO INHERIT
constraints. So this can only happen on a standalone table, and thus
changing the existing not-null constraint from NO INHERIT to normal does
no harm.

I think we could make CHECK behave the same way on this point; but in the
meantime, I propose this text:

If any of the <literal>CHECK</literal> constraints of the table being
attached are marked <literal>NO INHERIT</literal>, the command will fail;
such constraints must be recreated without the
<literal>NO INHERIT</literal> clause.
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) 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.

Speaking of pg_dump, I'm still on the nightmarish trip to get it to
behave correctly for all cases (esp. for pg_upgrade). It seems I
tripped up on my own code from the previous round, having
under-commented and misunderstood it :-(

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"The eagle never lost so much time, as
when he submitted to learn of the crow." (William Blake)