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.