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.