Re: NOT ENFORCED constraint feature

On Tue, Jan 28, 2025 at 4:01 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 20.01.25 17:53, Amul Sul wrote:
> >> Attached is a new set of patches. Please ignore patch 0001 here, which
> >> was posted separately [1] -- proposes allowing invalid foreign key
> >> constraints on partitioned tables. Patch 0002 refactors
> >> tryAttachPartitionForeignKey(), primarily needed by the new patch
> >> v9-0006 included in this set. This patch handles merging constraints
> >> with different enforceability. Without it, a new constraint would be
> >> created on the child. However, this patch introduces additional code
> >> that may appear somewhat messy or confusing. I've added plenty of
> >> comments to clarify the logic. While I’ve done some testing, it hasn’t
> >> been extensive. I plan to do more testing in the next week.
> >>
> >> Please let me know if we should continue with patch 0006 improvement,
> >> or if the feature up to patch 0005, which enforces matching
> >> enforceability before merging constraints, is sufficient.
> >>
> >> 1] https://postgr.es/m/CAAJ_b96Bp=-ZwihPPtuaNX=SrZ0U6ZsXD3+fgARO0JuKa8v2jQ@mail.gmail.com
> >>
> >
> > I made minor updates to the attached version, particularly ensuring
> > that the order of relation opening and closing remains the same as
> > before in ATExecAlterConstrRecurse(). Additionally, I’ve added a
> > refactoring patch to make createForeignKeyActionTriggers() accept a
> > relation OID instead of a Relation, making this function consistent
> > with others like createForeignKeyCheckTriggers().
>
> I think v10-0001 has been committed separately now. I can't
> successfully apply the remaining patches though. Could you supply an
> updated patch set?
>

Sure, I plan to work on that tomorrow.

> Just from looking at them, the refactoring patches 0002, 0003, 0004 seem ok.
>
> 0005 also seems ok.
>

Thank you.

>
> In 0006, this change in the test output should be improved:
>
> -- XXX: error message is misleading here
> ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
> -ERROR: ALTER CONSTRAINT statement constraints cannot be marked ENFORCED
> -LINE 1: ...TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
> - ^
> +ERROR: constraint "unique_tbl_i_key" of relation "unique_tbl" is not a
> foreign key constraint
>
> Maybe this should be along the lines of "ALTER CONSTRAINT ... ENFORCED
> is not supported for %s constraints" or something like that.
>

Ok, let me see what can be done here.

>
> This behavior is not correct:
>
> +-- Changing it back to ENFORCED will leave the constraint in the NOT
> VALID state
> +ALTER TABLE FKTABLE ALTER CONSTRAINT fktable_ftest1_fkey ENFORCED;
> +-- Which needs to be explicitly validated.
> +ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_ftest1_fkey;
>
> Setting the constraint to enforced should enforce it immediately. This
> SQL statement is covered by the SQL standard. Also, I think it's a
> better user experience if you don't require two steps.
>

Let me clarify: the constraint will be enforced for new inserts and
updates, but it won't be validated against existing data, so those
will remain marked as invalid.

Regards,