Re: NOT ENFORCED constraint feature

On 25.03.25 17:48, Peter Eisentraut wrote:
> I have committed patches 0001 through 0003.  I made some small changes:

> I will work through the remaining patches.  It looks good to me so far.

For the time being, here are the remaining patches rebased.

I think you could squash these together at this point.  This is 
especially useful since 0003 overwrites part of the changes in 0002, so 
it's better to see them all together.

Some details:

In CloneFkReferenced() and also in DetachPartitionFinalize(), you have 
this change:

-       fkconstraint->initially_valid = true;
+       fkconstraint->initially_valid = constrForm->convalidated;

I'm having a hard time understanding this.  Is this an pre-existing 
problem?  What does this change do?

Most of the other stuff is mechanical and fits into existing structures, 
so it seems ok.

Small cosmetic suggestion: write count(*) instead of count(1).  This 
fits better with existing practices.

The merge rules for inheritance and partitioning are still confusing.  I 
don't understand the behavior inh_check_constraint10 in the inherit.sql 
test:

+-- the invalid state of the child constraint will be ignored here.
+alter table p1 add constraint inh_check_constraint10 check (f1 < 10) 
not enforced;
+alter table p1_c1 add constraint inh_check_constraint10 check (f1 < 10) 
not valid enforced;

Why is that correct?  At least we should explain it here, or somewhere.

I'm also confused about the changes of the constraint names in the 
foreign_key.sql test:

-ERROR:  insert or update on table "fk_partitioned_fk_2" violates 
foreign key constraint "fk_partitioned_fk_a_b_fkey"
+ERROR:  insert or update on table "fk_partitioned_fk_2" violates 
foreign key constraint "fk_partitioned_fk_2_a_b_fkey"

And then patch 0003 changes it again.  This seems pretty random.  We 
should make sure that tests don't contain unrelated changes like that. 
(Or at least it's not clear why they are related.)

There is also in 0002

+-- should be having two constraints

and then in 0003:

--- should be having two constraints
+-- should only have one constraint

So another reason for squashing these together, so we don't have 
confusing intermediate states.

That said, is there a simpler way?  Patch 0003 appears to add a lot of 
complexity.  Could we make this simpler by saying, if you have otherwise 
matching constraints with different enforceability, make this an error. 
Then users can themselves adjust the enforceability how they want to 
make it match.