Re: NOT ENFORCED constraint feature

On Wed, Mar 26, 2025 at 9:33 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:

> On Tue, Mar 25, 2025 at 10:18 PM Peter Eisentraut <peter(at)eisentraut(dot)org>
> wrote:
> >
> > On 21.03.25 06:58, Amul Sul wrote:
> > >
> > > [....]
> > > Attached is the updated version, where the commit messages for patch
> > > 0005 and 0006 have been slightly corrected. Additionally, a few code
> > > comments have been updated to consistently use the ENFORCED/NOT
> > > ENFORCED keywords. The rest of the patches and all the code are
> > > unchanged.
> >
> > I have committed patches 0001 through 0003. I made some small changes:
> >
>
> Thank you very much !
>
> I wanted to run my dump and restore test on this patch set but it doesn't
apply cleanly. I tried applying 0004-0006. 0004 applies but not 0005.

I reviewed the tests to see if they leave objects in enough varied states
for 002_pg_upgrade to test it well. The test frequently drops and recreates
same partitioned and non-partitioned table inducing different states. So I
have a feeling that we are just leaving one state combination behind and
not all. It's an existing problem but probably with enforced and valid
states it becomes more serious. I ended up reviewing the tests. Here are
some comments.

+-- Reverting it back to ENFORCED will result in failure because constraint
validation will be triggered,
+-- as it was previously in a valid state.
+ALTER TABLE FKTABLE ALTER CONSTRAINT fktable_ftest1_fkey ENFORCED;
+ERROR: insert or update on table "fktable" violates foreign key
constraint "fktable_ftest1_fkey"
+DETAIL: Key (ftest1)=(1) is not present in table "pktable".

The text of error is misleading. There's no INSERT or UPDATE happening it's
ALTER. That's what VALIDATE CONSTRAINT also reports, so not fault of this
patch. But thought of writing it here since I noticed this now.

+-- Remove any existing rows that violate the constraint, then attempt to
change
+-- it.
+TRUNCATE FKTABLE;

I think, it will be good if we don't remove all the data; we wouldn't know
whether constraint was considered validated because there's no data or it
actually scanned the table and found no rows violating the constraint.
Let's leave/insert one row which obeys the constraint.

+-- Modifying other attributes of a constraint should not affect its
enforceability, and vice versa
+ALTER TABLE FKTABLE ADD CONSTRAINT fk_con FOREIGN KEY(ftest1, ftest2)
REFERENCES PKTABLE NOT VALID NOT ENFORCED;
+ALTER TABLE FKTABLE ALTER CONSTRAINT fk_con DEFERRABLE INITIALLY DEFERRED;
+SELECT condeferrable, condeferred, conenforced, convalidated
+FROM pg_constraint WHERE conname = 'fk_con';
+ condeferrable | condeferred | conenforced | convalidated
+---------------+-------------+-------------+--------------
+ t | t | f | f
+(1 row)
+
+ALTER TABLE FKTABLE ALTER CONSTRAINT fk_con NOT ENFORCED;
+SELECT condeferrable, condeferred, conenforced, convalidated
+FROM pg_constraint WHERE conname = 'fk_con';
+ condeferrable | condeferred | conenforced | convalidated
+---------------+-------------+-------------+--------------
+ t | t | f | f
+(1 row)

The statement is marking a NOT ENFORCED constraint as NOT ENFORCED. What is
it trying to test?

+
+ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT ENFORCED;
+BEGIN;
+-- doesn't match FK, no error.

A "but" before no error would make the comment clearer.

+UPDATE pktable SET id = 10 WHERE id = 5;
+-- doesn't match PK, no error.

A "but" before no error would make the comment clearer.

CREATE TABLE fk_partitioned_fk_1 (fdrop1 int, fdrop2 int, a int, fdrop3
int, b int);
ALTER TABLE fk_partitioned_fk_1 DROP COLUMN fdrop1, DROP COLUMN fdrop2,
DROP COLUMN fdrop3;
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_1 FOR
VALUES FROM (0,0) TO (1000,1000);
-ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES
fk_notpartitioned_pk;
+ALTER TABLE fk_partitioned_fk ADD CONSTRAINT fk_partitioned_fk_a_b_fkey
FOREIGN KEY (a, b)
+ REFERENCES fk_notpartitioned_pk NOT ENFORCED;
CREATE TABLE fk_partitioned_fk_2 (b int, fdrop1 int, fdrop2 int, a int);
ALTER TABLE fk_partitioned_fk_2 DROP COLUMN fdrop1, DROP COLUMN fdrop2;
+ALTER TABLE fk_partitioned_fk_2 ADD FOREIGN KEY (a, b) REFERENCES
fk_notpartitioned_pk NOT ENFORCED;
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR
VALUES FROM (1000,1000) TO (2000,2000);
+ALTER TABLE fk_partitioned_fk ALTER CONSTRAINT fk_partitioned_fk_a_b_fkey
ENFORCED;

I don't understand what value this change is bringing? Maybe a comment
explaining it. Why did we change name of one constraint and not the other?

ERROR: insert or update on table "fk_partitioned_fk_1" violates foreign
key constraint "fk_partitioned_fk_a_b_fkey"
DETAIL: Key (a, b)=(500, 501) is not present in table
"fk_notpartitioned_pk".
INSERT INTO fk_partitioned_fk (a,b) VALUES (1500, 1501);
-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"
DETAIL: Key (a, b)=(1500, 1501) is not present in table
"fk_notpartitioned_pk".
INSERT INTO fk_partitioned_fk_2 (a,b) VALUES (1500, 1501);
-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"
DETAIL: Key (a, b)=(1500, 1501) is not present in table
"fk_notpartitioned_pk".
INSERT INTO fk_partitioned_fk (a,b) VALUES (2500, 2502);
ERROR: insert or update on table "fk_partitioned_fk_3_1" violates foreign
key constraint "fk_partitioned_fk_a_b_fkey"

These diffs would go away if we didn't rename the constraint.

@@ -1667,6 +1753,37 @@ Indexes:
Referenced by:
TABLE "fk_partitioned_fk" CONSTRAINT "fk_partitioned_fk_a_b_fkey"
FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk(a, b)

+-- Check the exsting FK trigger
+CREATE TEMP TABLE tmp_trg_info AS SELECT tgtype, tgrelid::regclass,
tgconstraint
+FROM pg_trigger
+WHERE tgrelid IN (SELECT relid FROM
pg_partition_tree('fk_partitioned_fk'::regclass)
+ UNION ALL SELECT 'fk_notpartitioned_pk'::regclass);
+SELECT count(1) FROM tmp_trg_info;
+ count
+-------
+ 14
+(1 row)
+
+ALTER TABLE fk_partitioned_fk ALTER CONSTRAINT fk_partitioned_fk_a_b_fkey
NOT ENFORCED;
+-- No triggers
+SELECT count(1) FROM pg_trigger
+WHERE tgrelid IN (SELECT relid FROM
pg_partition_tree('fk_partitioned_fk'::regclass)
+ UNION ALL SELECT 'fk_notpartitioned_pk'::regclass);
+ count
+-------
+ 0
+(1 row)
+
+-- Changing it back to ENFORCED will recreate the necessary triggers.
+ALTER TABLE fk_partitioned_fk ALTER CONSTRAINT fk_partitioned_fk_a_b_fkey
ENFORCED;
+-- Should be exactly the same number of triggers found as before
+SELECT COUNT(1) FROM pg_trigger pgt JOIN tmp_trg_info tt
+ON (pgt.tgtype = tt.tgtype AND pgt.tgrelid = tt.tgrelid AND
pgt.tgconstraint = tt.tgconstraint);
+ count
+-------
+ 14
+(1 row)

Why don't we just print the relevant triggers. Just matching counts could
be misleading - we may have added one and dropped another keeping the count
same.

+BEGIN;
+ALTER TABLE fk_partitioned_fk ALTER CONSTRAINT fk_partitioned_fk_a_b_fkey
NOT ENFORCED;
+ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR
VALUES IN (1500,1502);
+-- should have only one constraint
+\d fk_partitioned_fk_2
+ Table "public.fk_partitioned_fk_2"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ b | integer | | |
+ a | integer | | |
+Partition of: fk_partitioned_fk FOR VALUES IN (1500, 1502)
+Foreign-key constraints:
+ TABLE "fk_partitioned_fk" CONSTRAINT "fk_partitioned_fk_a_b_fkey"
FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk(a, b) ON UPDATE CASCADE
ON DELETE CASCADE NOT ENFORCED

Did it just rename an existing constraint on fk_partitioned_fk_2? Is that
ok? I thought we just mark the constraint as inherited.

--
Best Wishes,
Ashutosh Bapat