Re: CHECK Constraint Deferrable - Mailing list pgsql-hackers

From Himanshu Upadhyaya
Subject Re: CHECK Constraint Deferrable
Date
Msg-id CAPF61jC8wbc+PnqcY=WbOM4NFxswatYWe2zsQWaE4ma8_1AYtQ@mail.gmail.com
Whole thread Raw
In response to Re: CHECK Constraint Deferrable  (Dean Rasheed <dean.a.rasheed@gmail.com>)
List pgsql-hackers


On Tue, Sep 19, 2023 at 4:14 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> I think we should be able to defer one constraint like in the case of
> foreign key constraint
>

Agreed. It should be possible to have a mix of deferred and immediate
constraint checks. In the example, the tbl_chk_1 is set deferred, but
it fails immediately, which is clearly not right.

Fixed in V3 patch. 
I would say that it's reasonable to limit the scope of this patch to
table constraints only, and leave domain constraints to a possible
follow-up patch.

Sure, Agree. 
A few other review comments:

1. The following produces a WARNING (possibly the same issue already reported):

CREATE TABLE foo (a int, b int);
ALTER TABLE foo ADD CONSTRAINT a_check CHECK (a > 0);
ALTER TABLE foo ADD CONSTRAINT b_check CHECK (b > 0) DEFERRABLE;

WARNING:  unexpected pg_constraint record found for relation "foo"

fixed in V3 patch. 
2. I think that equalTupleDescs() should compare the new fields, when
comparing the 2 sets of check constraints.

Fixed in V3 patch. 
3. The constraint exclusion code in the planner should ignore
deferrable check constraints (see get_relation_constraints() in
src/backend/optimizer/util/plancat.c), otherwise it might incorrectly
exclude a relation on the basis of a constraint that is temporarily
violated, and return incorrect query results. For example:

CREATE TABLE foo (a int);
CREATE TABLE foo_c1 () INHERITS (foo);
CREATE TABLE foo_c2 () INHERITS (foo);
ALTER TABLE foo_c2 ADD CONSTRAINT cc CHECK (a != 5) INITIALLY DEFERRED;

BEGIN;
INSERT INTO foo_c2 VALUES (5);
SET LOCAL constraint_exclusion TO off;
SELECT * FROM foo WHERE a = 5;
SET LOCAL constraint_exclusion TO on;
SELECT * FROM foo WHERE a = 5;
ROLLBACK;

Fixed in V3 patch. 
4. The code in MergeWithExistingConstraint() should prevent inherited
constraints being merged if their deferrable properties don't match
(as MergeConstraintsIntoExisting() does, since
constraints_equivalent() tests the deferrable fields). I.e., the
following should fail to merge the constraints, since they don't
match:

DROP TABLE IF EXISTS p,c;

CREATE TABLE p (a int, b int);
ALTER TABLE p ADD CONSTRAINT c1 CHECK (a > 0) DEFERRABLE;
ALTER TABLE p ADD CONSTRAINT c2 CHECK (b > 0);

CREATE TABLE c () INHERITS (p);
ALTER TABLE c ADD CONSTRAINT c1 CHECK (a > 0);
ALTER TABLE c ADD CONSTRAINT c2 CHECK (b > 0) DEFERRABLE;

I.e., that should produce an error, as happens if c is made to inherit
p *after* the constraints have been added.

Fixed in V3 patch.
5. Instead of just adding the new fields to the end of the ConstrCheck
struct, and to the end of lists of function parameters like
StoreRelCheck(), and other related code, it would be more logical to
put them immediately before the valid/invalid entries, to match the
order of constraint properties in pg_constraint, and functions like
CreateConstraintEntry().

Fixed in V3 patch.

--
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Himanshu Upadhyaya
Date:
Subject: Re: CHECK Constraint Deferrable
Next
From: Himanshu Upadhyaya
Date:
Subject: Re: CHECK Constraint Deferrable