Re: NOT ENFORCED constraint feature

Peter Eisentraut <peter@eisentraut.org>

From: Peter Eisentraut <peter@eisentraut.org>
To: Amul Sul <sulamul@gmail.com>
Cc: Álvaro Herrera <alvherre@alvh.no-ip.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, jian he <jian.universality@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Joel Jacobson <joel@compiler.org>, Alexandra Wang <alexandra.wang.oss@gmail.com>, Suraj Kharage <suraj.kharage@enterprisedb.com>
Date: 2025-03-11T17:43: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

I have committed the first three refactoring patches (v16-0001, 
v16-0002, v16-0003).  (I guess Álvaro didn't like the first one, so I 
suppose I'll revert that one, but it's a simple one, so you can proceed 
either way.)

I think the next step here is that you work to fix Álvaro's concerns 
about the recursion structure.

I have a few other review comments here in the meantime:

* patch v16-0007 "Ease the restriction that a NOT ENFORCED constraint 
must be INVALID."

I don't understand the purpose of this one.  And the commit message also 
doesn't explain the reason, only what it does.  I think we have settled 
on three states (not enforced and not valid; enforced but not yet valid; 
enforced and valid), so it seems sensible to keep valid as false if 
enforced is also false.  Did I miss something?

Specifically, this test case change

-ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK 
(b > 10) NOT ENFORCED; -- succeeds
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK 
(b > 10) NOT ENFORCED; -- fail
+ERROR:  check constraint "b_greater_than_ten_not_enforced" of relation 
"attmp3" is violated by some row
+ALTER TABLE attmp3 ADD CONSTRAINT b_greater_than_ten_not_enforced CHECK 
(b > 10) NOT VALID NOT ENFORCED; -- succeeds

seems very wrong to me.

* doc/src/sgml/advanced.sgml

Let's skip that.  This material is too advanced for a tutorial.

* doc/src/sgml/ddl.sgml

Let's move the material about NOT ENFORCED into a separate section or
paragraph, not in the first paragraph that introduces foreign keys.  I
suggest a separate sect2-level section at the end of the "Constraints"
section.

* src/backend/catalog/sql_features.txt

The SQL standard has NOT ENFORCED only for check and foreign-key
constraints, so you could flip this to "YES" here.  (Hmm, do we need
to support not-null constraints, though (which are grouped under check
constraints in the standard)?  Maybe turn the comment around and say
"except not-null constraints" or something like that.)

* src/backend/commands/tablecmds.c

I would omit this detail message:

errdetail("Enforceability can only be altered for foreign key constraints.")

We have generally tried to get rid of detail messages that say "cannot 
do this on this object type, but you could do it on a different object 
type", since that is not actually useful.

* src/test/regress/expected/foreign_key.out

This error message is confusing, since no insert or update is happening:

+ALTER TABLE FKTABLE ALTER CONSTRAINT fktable_ftest1_fkey ENFORCED;
+ERROR:  insert or update on table "fktable" violates foreign key 
constraint "fktable_ftest1_fkey"

Could we give a differently worded error message in this case?

Here, you are relying on the automatic constraint naming, which seems 
fragile and confusing:

+ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1, ftest2) REFERENCES PKTABLE 
NOT VALID NOT ENFORCED;
+ALTER TABLE FKTABLE ALTER CONSTRAINT fktable_ftest1_ftest2_fkey 
DEFERRABLE INITIALLY DEFERRED;

Better name the constraint explicitly in the first command.