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.


Reply via email to