Horton

Re: NOT ENFORCED constraint feature

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Amul Sul <sulamul@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, pgsql-hackers <pgsql-hackers@postgresql.org>, Joel Jacobson <joel@compiler.org>
Date: 2025-01-28T10:31:48Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for NOT ENFORCED in foreign key constraints

  2. Expand test a bit

  3. refactor: Pass relation OID instead of Relation to createForeignKeyCheckTriggers()

  4. refactor: Split ATExecAlterConstraintInternal()

  5. refactor: Move some code that updates pg_constraint to a separate function

  6. Move RemoveInheritedConstraint() call slightly earlier

  7. refactor: Split tryAttachPartitionForeignKey()

  8. refactor: re-add ATExecAlterChildConstr()

  9. Add ATAlterConstraint struct for ALTER .. CONSTRAINT

  10. refactor: split ATExecAlterConstrRecurse()

  11. Add support for NOT ENFORCED in CHECK constraints

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?

Just from looking at them, the refactoring patches 0002, 0003, 0004 seem ok.

0005 also seems ok.


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.


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.