Re: doc fail about ALTER TABLE ATTACH re. NO INHERIT

On 2024-Nov-13, Amit Langote wrote:

> I rebased my patch over 14e87ffa5c54 and noticed that NOT NULL
> constraints can also (or not) be marked NO INHERIT. I think we should
> allow NO INHERIT NOT NULL constraints on leaf partitions just like
> CHECK constraints, so changed AddRelationNotNullConstraints() that way
> and added some tests.

OK, looks good.

> However, I noticed that there is no clean way to do that for the following case:
>
> ALTER TABLE leaf_partition ADD CONSTRAINT col_nn_ni NOT NULL col NO INHERIT;

Sorry, I didn't understand what's the initial state. Does the
constraint already exist here or not?

> If I change the new function AdjustNotNullInheritance() added by your
> commit to not throw an error for leaf partitions, the above constraint
> (col_nn_ni) is not stored at all, because the function returns true in
> that case, which means the caller does nothing with the constraint. I
> am not sure what the right thing to do would be. If we return false,
> the caller will store the constraint the first time around, but
> repeating the command will again do nothing, not even prevent the
> addition of a duplicate constraint.

Do you mean if we return false, it allows two not-null constraints for
the same column? That would absolutely be a bug.

I think:
* if a leaf partition already has an inherited not-null constraint
from its parent and we want to add another one, we should:
- if the one being added is NO INHERIT, then throw an error, because
we cannot merge them
- if the one being added is not NO INHERIT, then both constraints
would have the same state and so we silently do nothing.
* if a leaf partition has a locally defined not-null marked NO INHERIT
- if we add another NO INHERIT, silently do nothing
- if we add an INHERIT one, throw an error because cannot merge.

If you want, you could leave the not-null constraint case alone and I
can have a look later. It wasn't my intention to burden you with that.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.