CHECK Constraint Deferrable

Started by Himanshu Upadhyayaalmost 3 years ago28 messageshackers
Jump to latest
#1Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com

Hi,

Currently, there is no support for CHECK constraint DEFERRABLE in a create
table statement.
SQL standard specifies that CHECK constraint can be defined as DEFERRABLE.

The attached patch is having implementation for CHECK constraint Deferrable
as below:

‘postgres[579453]=#’CREATE TABLE t1 (i int CHECK(i<>0) DEFERRABLE, t text);
CREATE TABLE
‘postgres[579453]=#’\d t1
Table "public.t1"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
i | integer | | |
t | text | | |
Check constraints:
"t1_i_check" CHECK (i <> 0) DEFERRABLE

Now we can have a deferrable CHECK constraint, and we can defer the
constraint validation:

‘postgres[579453]=#’BEGIN;
BEGIN
‘postgres[579453]=#*’SET CONSTRAINTS t1_i_check DEFERRED;
SET CONSTRAINTS
‘postgres[579453]=#*’INSERT INTO t1 VALUES (0, 'one'); -- should succeed
INSERT 0 1
‘postgres[579453]=#*’UPDATE t1 SET i = 1 WHERE t = 'one';
UPDATE 1
‘postgres[579453]=#*’COMMIT; -- should succeed
COMMIT

Attaching the initial patch, I will improve it with documentation in my
next version of the patch.

thoughts?

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

Attachments:

v1-0001-Implementation-of-CHECK-Constraint-to-make-it-Def.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Implementation-of-CHECK-Constraint-to-make-it-Def.patchDownload+518-47
#2Dilip Kumar
dilipbalaut@gmail.com
In reply to: Himanshu Upadhyaya (#1)
Re: CHECK Constraint Deferrable

On Wed, Jul 5, 2023 at 3:08 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Hi,

Currently, there is no support for CHECK constraint DEFERRABLE in a create table statement.
SQL standard specifies that CHECK constraint can be defined as DEFERRABLE.

I think this is a valid argument that this is part of SQL standard so
it would be good addition to PostgreSQL. So +1 for the feature.

But I am wondering whether there are some real-world use cases for
deferred CHECK/NOT NULL constraints? I mean like for foreign key
constraints if there is a cyclic dependency between two tables then
deferring the constraint is the simplest way to insert without error.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

#3Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: Dilip Kumar (#2)
Re: CHECK Constraint Deferrable

I can think of one scenario, as below

1) any department should have an employee
2)any employee should be assigned to a department
so, the employee table has a FK to the department table, and another check
constraint should be added to the department table to ensure there should
be one/more employees in this department. It's kind of a deadlock
situation, each one depends on the other one. We cant insert a new
department, coz there is no employee. Also, we can't insert new employee
belongs to this new department, coz the department hasn't been and cant be
added. So if we have a check constraint defined as deferrable we can solve
this problem.

‘postgres[685143]=#’CREATE FUNCTION checkEmpPresent(did int) RETURNS int AS
$$ SELECT count(*) from emp where emp.deptno = did $$ IMMUTABLE LANGUAGE
SQL;
CREATE FUNCTION
‘postgres[685143]=#’alter table dept add constraint check_cons check
(checkEmpPresent(deptno) > 0);
ALTER TABLE
‘postgres[685143]=#’\d dept;
Table "public.dept"
Column | Type | Collation | Nullable | Default
----------+---------------+-----------+----------+---------
deptno | integer | | not null |
deptname | character(20) | | |
Indexes:
"dept_pkey" PRIMARY KEY, btree (deptno)
Check constraints:
"check_cons" CHECK (checkemppresent(deptno) > 0)
Referenced by:
TABLE "emp" CONSTRAINT "fk_cons" FOREIGN KEY (deptno) REFERENCES
dept(deptno)

‘postgres[685143]=#’insert into dept values (1, 'finance');
ERROR: 23514: new row for relation "dept" violates check constraint
"check_cons"
DETAIL: Failing row contains (1, finance ).
SCHEMA NAME: public
TABLE NAME: dept
CONSTRAINT NAME: check_cons
LOCATION: ExecConstraints, execMain.c:2069
‘postgres[685143]=#’\d emp;
Table "public.emp"
Column | Type | Collation | Nullable | Default
--------+---------------+-----------+----------+---------
empno | integer | | |
ename | character(20) | | |
deptno | integer | | |
Foreign-key constraints:
"fk_cons" FOREIGN KEY (deptno) REFERENCES dept(deptno)

‘postgres[685143]=#’insert into emp values (1001, 'test', 1);
ERROR: 23503: insert or update on table "emp" violates foreign key
constraint "fk_cons"
DETAIL: Key (deptno)=(1) is not present in table "dept".
SCHEMA NAME: public
TABLE NAME: emp
CONSTRAINT NAME: fk_cons
LOCATION: ri_ReportViolation, ri_triggers.c:2608

I have tried with v1 patch as below;

‘postgres[685143]=#’alter table dept drop constraint check_cons;
ALTER TABLE
‘postgres[685143]=#’alter table dept add constraint check_cons check
(checkEmpPresent(deptno) > 0) DEFERRABLE INITIALLY DEFERRED;
ALTER TABLE
‘postgres[685143]=#’BEGIN;
BEGIN
‘postgres[685143]=#*’insert into dept values (1, 'finance');
INSERT 0 1
‘postgres[685143]=#*’insert into emp values (1001, 'test', 1);
INSERT 0 1
‘postgres[685143]=#*’commit;
COMMIT
‘postgres[685143]=#’select * from dept;
deptno | deptname
--------+----------------------
1 | finance
(1 row)

‘postgres[685143]=#’select * from emp;
empno | ename | deptno
-------+----------------------+--------
1001 | test | 1
(1 row)

Thanks,
Himanshu

On Fri, Jul 7, 2023 at 5:21 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Wed, Jul 5, 2023 at 3:08 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Hi,

Currently, there is no support for CHECK constraint DEFERRABLE in a

create table statement.

SQL standard specifies that CHECK constraint can be defined as

DEFERRABLE.

I think this is a valid argument that this is part of SQL standard so
it would be good addition to PostgreSQL. So +1 for the feature.

But I am wondering whether there are some real-world use cases for
deferred CHECK/NOT NULL constraints? I mean like for foreign key
constraints if there is a cyclic dependency between two tables then
deferring the constraint is the simplest way to insert without error.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

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

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Himanshu Upadhyaya (#3)
Re: CHECK Constraint Deferrable

On Friday, July 7, 2023, Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com>
wrote:

I can think of one scenario, as below

1) any department should have an employee
2)any employee should be assigned to a department
so, the employee table has a FK to the department table, and another check
constraint should be added to the department table to ensure there should
be one/more employees in this department.

That isn’t a valid/allowed check constraint - it contains a prohibited
reference to another table.

David J.

#5Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: David G. Johnston (#4)
Re: CHECK Constraint Deferrable

Attached is v2 of the patch, rebased against the latest HEAD.

Thanks,
Himanshu

Attachments:

v2-0001-Implementation-of-CHECK-Constraint-to-make-it.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Implementation-of-CHECK-Constraint-to-make-it.patchDownload+518-47
#6Dilip Kumar
dilipbalaut@gmail.com
In reply to: Himanshu Upadhyaya (#5)
Re: CHECK Constraint Deferrable

On Thu, Sep 7, 2023 at 1:25 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Attached is v2 of the patch, rebased against the latest HEAD.

I have done some initial reviews, and here are my comments. More
detailed review later. Meanwhile, you can work on these comments and
fix all the cosmetics especially 80 characters per line

1.
+
+ (void) CreateTrigger(trigger, NULL, RelationGetRelid(rel),
+ InvalidOid, constrOid, InvalidOid, InvalidOid,
+ InvalidOid, NULL, true, false);

heap.c is calling CreateTrigger but the inclusion of
"commands/trigger.h" is missing.

2.
- if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
+ if ((failed = ExecRelCheck(resultRelInfo, slot, estate,
checkConstraint, &recheckConstraints)) != NULL && !recheckConstraints)

Why recheckConstraints need to get as output from ExecRelCheck? I
mean whether it will be rechecked or nor is already known by the
caller and
Whether the constraint failed or passed is known based on the return
value so why do you need to extra parameter here?

3.
-void
+bool
 ExecConstraints(ResultRelInfo *resultRelInfo,
- TupleTableSlot *slot, EState *estate)
+ TupleTableSlot *slot, EState *estate, checkConstraintRecheck checkConstraint)
 {
- if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
+ if ((failed = ExecRelCheck(resultRelInfo, slot, estate,
checkConstraint, &recheckConstraints)) != NULL && !recheckConstraints)

take care of postgres coding style and break line after 80
characters. Check other places as well in the patch.

4.
+ if (checkConstraint == CHECK_RECHECK_ENABLED && check[i].ccdeferrable)
+ {
+ *recheckConstraints = true;
+ }

Remove curly brackets around single-line block

5.
+typedef enum checkConstraintRecheck
+{
+ CHECK_RECHECK_DISABLED, /* Recheck of CHECK constraint is disabled, so
+ * DEFERRED CHECK constraint will be
+ * considered as non-deferrable check
+ * constraint.  */
+ CHECK_RECHECK_ENABLED, /* Recheck of CHECK constraint is enabled, so
+ * CHECK constraint will be validated but
+ * error will not be reported for deferred
+ * CHECK constraint. */
+ CHECK_RECHECK_EXISTING /* Recheck of existing violated CHECK
+ * constraint, indicates that this is a
+ * deferred recheck of a row that was reported
+ * as a potential violation of CHECK
+ * CONSTRAINT */
+} checkConstraintRecheck;

I do not like the naming convention here, especially the words
RECHECK, DISABLE, and ENABLE. And also the name of the enum is a bit
off. We can name it more like a unique constraint
YES, PARTIAL, EXISTING

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

#7Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: Dilip Kumar (#6)
Re: CHECK Constraint Deferrable

On Fri, Sep 8, 2023 at 1:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

2.
- if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
+ if ((failed = ExecRelCheck(resultRelInfo, slot, estate,
checkConstraint, &recheckConstraints)) != NULL && !recheckConstraints)

Why recheckConstraints need to get as output from ExecRelCheck? I
mean whether it will be rechecked or nor is already known by the
caller and

Yes it will be known to the caller but ExecRelCheck will set this new
parameter only if any one of the constraint is defined as Deferrable (in
create table statement) and there is a potential constraint violation.

Whether the constraint failed or passed is known based on the return
value so why do you need to extra parameter here?

Because if normal CHECK constraint(non Deferrable) is violated, no need to

proceed with the insertion and in that case recheckConstraints will hold
"false" but if Deferrable check constraint is violated, we need to
revalidate the constraint at commit time and recheckConstraints will hold
"true".

5.
+typedef enum checkConstraintRecheck
+{
+ CHECK_RECHECK_DISABLED, /* Recheck of CHECK constraint is disabled, so
+ * DEFERRED CHECK constraint will be
+ * considered as non-deferrable check
+ * constraint.  */
+ CHECK_RECHECK_ENABLED, /* Recheck of CHECK constraint is enabled, so
+ * CHECK constraint will be validated but
+ * error will not be reported for deferred
+ * CHECK constraint. */
+ CHECK_RECHECK_EXISTING /* Recheck of existing violated CHECK
+ * constraint, indicates that this is a
+ * deferred recheck of a row that was reported
+ * as a potential violation of CHECK
+ * CONSTRAINT */
+} checkConstraintRecheck;

I do not like the naming convention here, especially the words
RECHECK, DISABLE, and ENABLE. And also the name of the enum is a bit
off. We can name it more like a unique constraint
YES, PARTIAL, EXISTING

I can think of alternative ENUM name as "EnforceDeferredCheck" and member

as “CHECK_DEFERRED_YES”, “CHECK_DEFRRED_NO” and “CHECK_DEFERRED_EXISTING”.

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

#8vignesh C
vignesh21@gmail.com
In reply to: Himanshu Upadhyaya (#5)
Re: CHECK Constraint Deferrable

On Thu, 7 Sept 2023 at 17:26, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Attached is v2 of the patch, rebased against the latest HEAD.

Thanks for working on this, few comments:
1) "CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t
text)" is crashing in windows, the same was noticed in CFBot too:
2023-09-11 08:11:36.585 UTC [58563][client backend]
[pg_regress/constraints][13/880:0] LOG: statement: CREATE TABLE
check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
2023-09-11 08:11:36.586 UTC [58560][client backend]
[pg_regress/inherit][15/391:0] LOG: statement: drop table c1;
../src/backend/commands/trigger.c:220:26: runtime error: member access
within null pointer of type 'struct CreateTrigStmt'
==58563==Using libbacktrace symbolizer.

The details of CFBot failure can be seen at [1]https://api.cirrus-ci.com/v1/artifact/task/4855966353588224/testrun/build-32/testrun/pg_upgrade/002_pg_upgrade/log/002_pg_upgrade_old_node.log

2) Alter of check constraint deferrable is not handled, is this intentional?
CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
postgres=# alter table check_constr_tbl alter constraint
check_constr_tbl_i_check not deferrable;
ERROR: constraint "check_constr_tbl_i_check" of relation
"check_constr_tbl" is not a foreign key constraint

3) Should we handle this scenario for domains too:
CREATE DOMAIN c1_check AS INT CHECK(VALUE > 10);
create table test(c1 c1_check);
alter domain c1_check ADD check (VALUE > 20) DEFERRABLE INITIALLY DEFERRED;

begin;
-- should this be deffered
insert into test values(19);
ERROR: value for domain c1_check violates check constraint "c1_check_check1"

4) There is one warning:
heap.c: In function ‘StoreRelCheck’:
heap.c:2178:24: warning: implicit declaration of function
‘CreateTrigger’ [-Wimplicit-function-declaration]
2178 | (void) CreateTrigger(trigger, NULL,
RelationGetRelid(rel),
| ^~~~~~~~~~~~~

5) This should be added to typedefs.list file:
+typedef enum checkConstraintRecheck
+{
+       CHECK_RECHECK_DISABLED,         /* Recheck of CHECK constraint
is disabled, so
+                                                                *
DEFERRED CHECK constraint will be
+                                                                *
considered as non-deferrable check
+                                                                *
constraint.  */
+       CHECK_RECHECK_ENABLED,          /* Recheck of CHECK constraint
is enabled, so
+                                                                *
CHECK constraint will be validated but
+                                                                *
error will not be reported for deferred
+                                                                *
CHECK constraint. */
+       CHECK_RECHECK_EXISTING          /* Recheck of existing violated CHECK
+                                                                *
constraint, indicates that this is a
+                                                                *
deferred recheck of a row that was reported
+                                                                * as
a potential violation of CHECK
+                                                                * CONSTRAINT */
+}                      checkConstraintRecheck;

[1]: https://api.cirrus-ci.com/v1/artifact/task/4855966353588224/testrun/build-32/testrun/pg_upgrade/002_pg_upgrade/log/002_pg_upgrade_old_node.log

Regards,
Vignesh

#9vignesh C
vignesh21@gmail.com
In reply to: Himanshu Upadhyaya (#5)
Re: CHECK Constraint Deferrable

On Thu, 7 Sept 2023 at 17:26, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Attached is v2 of the patch, rebased against the latest HEAD.

Few issues:
1) Create domain fails but alter domain is successful, I feel we
should support create domain too:
postgres=# create domain d1 as int check(value<>0) deferrable;
ERROR: specifying constraint deferrability not supported for domains
postgres=# create domain d1 as int check(value<>0);
CREATE DOMAIN
postgres=# alter domain d1 add constraint con_2 check(value<>1) deferrable;
ALTER DOMAIN

2) I was not sure, if the error message change was intentional:
2a)
In Head:
CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER s1;
ERROR: misplaced DEFERRABLE clause
LINE 1: CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER...
^
postgres=# CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER s1;
ERROR: "t9" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

2b)
In Head:
postgres=# CREATE FOREIGN TABLE t2(a int CHECK(a<>0)) SERVER s1;
CREATE FOREIGN TABLE
postgres=# ALTER FOREIGN TABLE t2 ADD CONSTRAINT t2_chk_1 CHECK(a<>1)
DEFERRABLE;
ERROR: CHECK constraints cannot be marked DEFERRABLE

With patch:
postgres=# ALTER FOREIGN TABLE t8 ADD CONSTRAINT t8_chk_1 CHECK(a<>1)
DEFERRABLE;
ERROR: "t8" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

3) Insert check is not deferred to commit:
This insert check here is deferred to commit:
postgres=# CREATE TABLE tbl (i int ) partition by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
INSERT 0 1
postgres=*# commit;
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

But the check here is not deferred to commit:
postgres=# CREATE TABLE tbl (i int check(i<>0) DEFERRABLE) partition
by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

4) There is a new warning popping up now:
CREATE TABLE tbl_new_3 (i int check(i<>0)) partition by range (i);
CREATE FOREIGN TABLE ftbl_new_3 PARTITION OF tbl_new_3 FOR VALUES FROM
(40) TO (50) server s1;
postgres=# ALTER TABLE tbl_new_3 ADD CONSTRAINT tbl_new_3_chk
CHECK(i<>1) DEFERRABLE;
WARNING: unexpected pg_constraint record found for relation "tbl_new_3"
ERROR: "ftbl_new_3" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

Regards,
Vignesh

#10Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: vignesh C (#8)
Re: CHECK Constraint Deferrable

Thanks for the review comments.

On Tue, Sep 12, 2023 at 2:56 PM vignesh C <vignesh21@gmail.com> wrote:

On Thu, 7 Sept 2023 at 17:26, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Attached is v2 of the patch, rebased against the latest HEAD.

Thanks for working on this, few comments:
1) "CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t
text)" is crashing in windows, the same was noticed in CFBot too:
2023-09-11 08:11:36.585 UTC [58563][client backend]
[pg_regress/constraints][13/880:0] LOG: statement: CREATE TABLE
check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
2023-09-11 08:11:36.586 UTC [58560][client backend]
[pg_regress/inherit][15/391:0] LOG: statement: drop table c1;
../src/backend/commands/trigger.c:220:26: runtime error: member access
within null pointer of type 'struct CreateTrigStmt'
==58563==Using libbacktrace symbolizer.

Will Fix this in my next patch.

The details of CFBot failure can be seen at [1]

2) Alter of check constraint deferrable is not handled, is this
intentional?
CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
postgres=# alter table check_constr_tbl alter constraint
check_constr_tbl_i_check not deferrable;
ERROR: constraint "check_constr_tbl_i_check" of relation
"check_constr_tbl" is not a foreign key constraint

ALTER CONSTRAINT is currently only supported for FOREIGN KEY, it's even

not supported for UNIQUE constraint as below:
‘postgres[1271421]=#’CREATE TABLE unique_constr_tbl (i int unique
DEFERRABLE, t text);
CREATE TABLE
‘postgres[1271421]=#’\d unique_constr_tbl;
Table "public.unique_constr_tbl"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
i | integer | | |
t | text | | |
Indexes:
"unique_constr_tbl_i_key" UNIQUE CONSTRAINT, btree (i) DEFERRABLE
‘postgres[1271421]=#’alter table unique_constr_tbl alter constraint
unique_constr_tbl_i_key not deferrable;
ERROR: 42809: constraint "unique_constr_tbl_i_key" of relation
"unique_constr_tbl" is not a foreign key constraint
LOCATION: ATExecAlterConstraint, tablecmds.c:11183

I still need to understand the design restriction here, please let me know
if anyone is aware of this?
is it because of dependency on Indexes?

3) Should we handle this scenario for domains too:

CREATE DOMAIN c1_check AS INT CHECK(VALUE > 10);
create table test(c1 c1_check);
alter domain c1_check ADD check (VALUE > 20) DEFERRABLE INITIALLY DEFERRED;

begin;
-- should this be deffered
insert into test values(19);
ERROR: value for domain c1_check violates check constraint
"c1_check_check1"

Yes, thanks for notifying, I missed this for CREATE DOMAIN, will analyse

and include in next revision.

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

#11Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: vignesh C (#9)
Re: CHECK Constraint Deferrable

On Thu, Sep 14, 2023 at 9:57 AM vignesh C <vignesh21@gmail.com> wrote:

3) Insert check is not deferred to commit:
This insert check here is deferred to commit:
postgres=# CREATE TABLE tbl (i int ) partition by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
INSERT 0 1
postgres=*# commit;
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

But the check here is not deferred to commit:
postgres=# CREATE TABLE tbl (i int check(i<>0) DEFERRABLE) partition
by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

I dont think it's a problem, in the second case there are two DEFERRABLE

CHECK constraints and you are marking one as DEFERRED but other one will be
INITIALLY IMMEDIATE. so we can use "SET CONSTRAINTS ALL DEFERRED;".
‘postgres[1271421]=#’CREATE TABLE tbl (i int check(i<>0) DEFERRABLE)
partition
‘...>’by range (i);
CREATE TABLE
‘postgres[1271421]=#’CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM
(0) TO (10);
CREATE TABLE
‘postgres[1271421]=#’CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM
(20) TO (30);
CREATE TABLE
‘postgres[1271421]=#’ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1)
DEFERRABLE;
ALTER TABLE
‘postgres[1271421]=#’\d tbl
Partitioned table "public.tbl"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
i | integer | | |
Partition key: RANGE (i)
Check constraints:
"tbl_chk_1" CHECK (i <> 1) DEFERRABLE
"tbl_i_check" CHECK (i <> 0) DEFERRABLE
Number of partitions: 2 (Use \d+ to list them.)
‘postgres[1271421]=#’begin;
BEGIN
‘postgres[1271421]=#*’SET CONSTRAINTS ALL DEFERRED;
SET CONSTRAINTS
‘postgres[1271421]=#*’INSERT INTO tbl values (1);
INSERT 0 1
‘postgres[1271421]=#*’commit;
ERROR: 23514: new row for relation "tbl_1" violates check constraint
"tbl_chk_1"
DETAIL: Failing row contains (1).
SCHEMA NAME: public
TABLE NAME: tbl_1
CONSTRAINT NAME: tbl_chk_1
LOCATION: ExecConstraints, execMain.c:2077

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

#12vignesh C
vignesh21@gmail.com
In reply to: Himanshu Upadhyaya (#11)
Re: CHECK Constraint Deferrable

On Thu, 14 Sept 2023 at 15:33, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

On Thu, Sep 14, 2023 at 9:57 AM vignesh C <vignesh21@gmail.com> wrote:

3) Insert check is not deferred to commit:
This insert check here is deferred to commit:
postgres=# CREATE TABLE tbl (i int ) partition by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
INSERT 0 1
postgres=*# commit;
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

But the check here is not deferred to commit:
postgres=# CREATE TABLE tbl (i int check(i<>0) DEFERRABLE) partition
by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

I dont think it's a problem, in the second case there are two DEFERRABLE CHECK constraints and you are marking one as DEFERRED but other one will be INITIALLY IMMEDIATE. so we can use "SET CONSTRAINTS ALL DEFERRED;".
‘postgres[1271421]=#’CREATE TABLE tbl (i int check(i<>0) DEFERRABLE) partition
‘...>’by range (i);
CREATE TABLE
‘postgres[1271421]=#’CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE
‘postgres[1271421]=#’CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
‘postgres[1271421]=#’ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
‘postgres[1271421]=#’\d tbl
Partitioned table "public.tbl"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
i | integer | | |
Partition key: RANGE (i)
Check constraints:
"tbl_chk_1" CHECK (i <> 1) DEFERRABLE
"tbl_i_check" CHECK (i <> 0) DEFERRABLE
Number of partitions: 2 (Use \d+ to list them.)
‘postgres[1271421]=#’begin;
BEGIN
‘postgres[1271421]=#*’SET CONSTRAINTS ALL DEFERRED;
SET CONSTRAINTS
‘postgres[1271421]=#*’INSERT INTO tbl values (1);
INSERT 0 1
‘postgres[1271421]=#*’commit;
ERROR: 23514: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).
SCHEMA NAME: public
TABLE NAME: tbl_1
CONSTRAINT NAME: tbl_chk_1
LOCATION: ExecConstraints, execMain.c:2077

I think we should be able to defer one constraint like in the case of
foreign key constraint:
create table t1(c1 int primary key);
insert into t1 values(10);
create table t2(c1 int primary key);
insert into t2 values(10);
create table t3(c1 int, c2 int references t1(c1) deferrable, c3 int
references t2(c1) deferrable);

-- Set only one constraint as deferred
begin;
set CONSTRAINTS t3_c2_fkey deferred;
-- c2 column constraint is deferred, we need not set all constraints
deferred in this case, insert was successful
postgres=*# insert into t3 values(1,11,10);
INSERT 0 1
-- Throws error for the constraint that is not deferred
postgres=*# insert into t3 values(1,10,11);
ERROR: insert or update on table "t3" violates foreign key constraint
"t3_c3_fkey"
DETAIL: Key (c3)=(11) is not present in table "t2".

Thoughts?

Regards,
Vignesh

#13Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: vignesh C (#12)
Re: CHECK Constraint Deferrable

On Fri, 15 Sept 2023 at 08:00, vignesh C <vignesh21@gmail.com> wrote:

On Thu, 14 Sept 2023 at 15:33, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

On Thu, Sep 14, 2023 at 9:57 AM vignesh C <vignesh21@gmail.com> wrote:

postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

I dont think it's a problem, in the second case there are two DEFERRABLE CHECK constraints and you are marking one as DEFERRED but other one will be INITIALLY IMMEDIATE.

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.

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.

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"

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

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;

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.

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().

Regards,
Dean

#14Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: vignesh C (#8)
Re: CHECK Constraint Deferrable

On Tue, Sep 12, 2023 at 2:56 PM vignesh C <vignesh21@gmail.com> wrote:

On Thu, 7 Sept 2023 at 17:26, Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:

Attached is v2 of the patch, rebased against the latest HEAD.

Thanks for working on this, few comments:
1) "CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t
text)" is crashing in windows, the same was noticed in CFBot too:
2023-09-11 08:11:36.585 UTC [58563][client backend]
[pg_regress/constraints][13/880:0] LOG: statement: CREATE TABLE
check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
2023-09-11 08:11:36.586 UTC [58560][client backend]
[pg_regress/inherit][15/391:0] LOG: statement: drop table c1;
../src/backend/commands/trigger.c:220:26: runtime error: member access
within null pointer of type 'struct CreateTrigStmt'
==58563==Using libbacktrace symbolizer.

The details of CFBot failure can be seen at [1]

I have tried it with my latest patch on windows environment and not

getting any crash with the above statement, will do further analysis if
this patch also has the same issue.

2) Alter of check constraint deferrable is not handled, is this
intentional?
CREATE TABLE check_constr_tbl (i int CHECK(i<>0) DEFERRABLE, t text);
postgres=# alter table check_constr_tbl alter constraint
check_constr_tbl_i_check not deferrable;
ERROR: constraint "check_constr_tbl_i_check" of relation
"check_constr_tbl" is not a foreign key constraint

This is not allowed for any constraint type but FOREIGN key. I am not very

sure about if there is any limitation with this so wanted to take opinion
from other hackers on this.

3) Should we handle this scenario for domains too:
CREATE DOMAIN c1_check AS INT CHECK(VALUE > 10);
create table test(c1 c1_check);
alter domain c1_check ADD check (VALUE > 20) DEFERRABLE INITIALLY DEFERRED;

begin;
-- should this be deffered
insert into test values(19);
ERROR: value for domain c1_check violates check constraint
"c1_check_check1"

We are planning to have a follow-up patch once this initial patch is

committed.

4) There is one warning:
heap.c: In function ‘StoreRelCheck’:
heap.c:2178:24: warning: implicit declaration of function
‘CreateTrigger’ [-Wimplicit-function-declaration]
2178 | (void) CreateTrigger(trigger, NULL,
RelationGetRelid(rel),
|

Fixed in V3 patch.

^~~~~~~~~~~~~

5) This should be added to typedefs.list file:
+typedef enum checkConstraintRecheck
+{
+       CHECK_RECHECK_DISABLED,         /* Recheck of CHECK constraint
is disabled, so
+                                                                *
DEFERRED CHECK constraint will be
+                                                                *
considered as non-deferrable check
+                                                                *
constraint.  */
+       CHECK_RECHECK_ENABLED,          /* Recheck of CHECK constraint
is enabled, so
+                                                                *
CHECK constraint will be validated but
+                                                                *
error will not be reported for deferred
+                                                                *
CHECK constraint. */
+       CHECK_RECHECK_EXISTING          /* Recheck of existing violated
CHECK
+                                                                *
constraint, indicates that this is a
+                                                                *
deferred recheck of a row that was reported
+                                                                * as
a potential violation of CHECK
+                                                                *
CONSTRAINT */
+}                      checkConstraintRecheck;

Fixed in V3 patch.

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

#15Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: vignesh C (#9)
Re: CHECK Constraint Deferrable

On Thu, Sep 14, 2023 at 9:57 AM vignesh C <vignesh21@gmail.com> wrote:

2) I was not sure, if the error message change was intentional:
2a)
In Head:
CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER s1;
ERROR: misplaced DEFERRABLE clause
LINE 1: CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER...
^
postgres=# CREATE FOREIGN TABLE t9(a int CHECK(a<>0) DEFERRABLE) SERVER s1;
ERROR: "t9" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

2b)
In Head:
postgres=# CREATE FOREIGN TABLE t2(a int CHECK(a<>0)) SERVER s1;
CREATE FOREIGN TABLE
postgres=# ALTER FOREIGN TABLE t2 ADD CONSTRAINT t2_chk_1 CHECK(a<>1)
DEFERRABLE;
ERROR: CHECK constraints cannot be marked DEFERRABLE

With patch:
postgres=# ALTER FOREIGN TABLE t8 ADD CONSTRAINT t8_chk_1 CHECK(a<>1)
DEFERRABLE;
ERROR: "t8" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

We are creating a constraint trigger for DEFERRED check constraint and as

per implementation of FOREIGN table we are restricting to have a constraint
trigger. I need to do more analysis before reaching to any conclusion, I
think we can restrict this gram.y itself.

3) Insert check is not deferred to commit:
This insert check here is deferred to commit:
postgres=# CREATE TABLE tbl (i int ) partition by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
INSERT 0 1
postgres=*# commit;
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

But the check here is not deferred to commit:
postgres=# CREATE TABLE tbl (i int check(i<>0) DEFERRABLE) partition
by range (i);
CREATE TABLE tbl_1 PARTITION OF tbl FOR VALUES FROM (0) TO (10);
CREATE TABLE tbl_2 PARTITION OF tbl FOR VALUES FROM (20) TO (30);
CREATE TABLE
CREATE TABLE
CREATE TABLE
postgres=# ALTER TABLE tbl ADD CONSTRAINT tbl_chk_1 CHECK(i<>1) DEFERRABLE;
ALTER TABLE
postgres=# begin;
BEGIN
postgres=*# SET CONSTRAINTS tbl_chk_1 DEFERRED;
SET CONSTRAINTS
postgres=*# INSERT INTO tbl values (1);
ERROR: new row for relation "tbl_1" violates check constraint "tbl_chk_1"
DETAIL: Failing row contains (1).

Fixed in V3 patch.

4) There is a new warning popping up now:
CREATE TABLE tbl_new_3 (i int check(i<>0)) partition by range (i);
CREATE FOREIGN TABLE ftbl_new_3 PARTITION OF tbl_new_3 FOR VALUES FROM
(40) TO (50) server s1;
postgres=# ALTER TABLE tbl_new_3 ADD CONSTRAINT tbl_new_3_chk
CHECK(i<>1) DEFERRABLE;
WARNING: unexpected pg_constraint record found for relation "tbl_new_3"
ERROR: "ftbl_new_3" is a foreign table
DETAIL: Foreign tables cannot have constraint triggers.

Fixed in V3 patch.

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

#16Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: Dean Rasheed (#13)
Re: CHECK Constraint Deferrable

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

#17Himanshu Upadhyaya
upadhyaya.himanshu@gmail.com
In reply to: Himanshu Upadhyaya (#16)
Re: CHECK Constraint Deferrable

On Mon, Oct 2, 2023 at 8:31 PM Himanshu Upadhyaya <
upadhyaya.himanshu@gmail.com> wrote:

V3 patch attached.

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

Attachments:

v3-0001-Implementation-of-CHECK-Constraint-to-make-it.patchapplication/octet-stream; name=v3-0001-Implementation-of-CHECK-Constraint-to-make-it.patchDownload+625-66
#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Himanshu Upadhyaya (#17)
Re: CHECK Constraint Deferrable

Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> writes:

V3 patch attached.

Sorry for not weighing in on this before, but ... is this a feature
we want at all? We are very clear in the existing docs that CHECK
conditions must be immutable [1]See Note at the bottom of "5.4.1. Check Constraints" here: https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-CHECK-CONSTRAINTS, and that's not something we can
easily relax because if they are not then it's unclear when we need
to recheck them to ensure they stay satisfied. But here we have a
feature whose only possible use is with constraints that *aren't*
immutable; else we might as well just check them immediately.
So that gives rise to a bunch of subtle questions about exactly what
properties a user-written constraint would need to have to guarantee
sane semantics given this implementation. Can we define what those
properties are, or what the ensuing semantic guarantees are exactly?
Can we explain those things clearly enough that the average user would
have a shot at writing a valid deferred constraint? Is a deferred
constraint having those properties likely to be actually useful?

I don't know the answers to these questions, but it troubles me a
lot that zero consideration appears to have been given to them.
I do not think we should put more effort into this patch unless
satisfactory answers are forthcoming.

regards, tom lane

[1]: See Note at the bottom of "5.4.1. Check Constraints" here: https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-CHECK-CONSTRAINTS
https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-CHECK-CONSTRAINTS

#19David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#18)
Re: CHECK Constraint Deferrable

On Mon, Oct 2, 2023 at 12:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> writes:

V3 patch attached.

Sorry for not weighing in on this before, but ... is this a feature
we want at all? We are very clear in the existing docs that CHECK
conditions must be immutable [1], and that's not something we can
easily relax because if they are not then it's unclear when we need
to recheck them to ensure they stay satisfied.

Agreed. I'm not sold on conforming to the standard being an appropriate
ideal here. Either we already don't because our check constraints are
immutable, or I'm missing what use case the committee had in mind when they
designed this feature. In any case, its absence doesn't seem that sorely
missed, and the OP's only actual example would require relaxing the
immutable property which I disagree with. We have deferrable triggers to
serve that posited use case.

David J.

#20Vik Fearing
vik@postgresfriends.org
In reply to: Tom Lane (#18)
Re: CHECK Constraint Deferrable

On 10/2/23 21:25, Tom Lane wrote:

Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> writes:

V3 patch attached.

Sorry for not weighing in on this before, but ... is this a feature
we want at all?

For standards conformance, I vote yes.

We are very clear in the existing docs that CHECK
conditions must be immutable [1], and that's not something we can
easily relax because if they are not then it's unclear when we need
to recheck them to ensure they stay satisfied.

That is what the *user* documentation says, but we both know it isn't true.

Here is a short conversation you and I had about five years ago where
you defended the non-immutability of CHECK constraints:
/messages/by-id/12539.1544107316@sss.pgh.pa.us

But here we have a
feature whose only possible use is with constraints that *aren't*
immutable; else we might as well just check them immediately.

I disagree with this. The whole point of deferring constraints is to be
able to do some cleanup before the consistency is checked.

So that gives rise to a bunch of subtle questions about exactly what
properties a user-written constraint would need to have to guarantee
sane semantics given this implementation. Can we define what those
properties are, or what the ensuing semantic guarantees are exactly?
Can we explain those things clearly enough that the average user would
have a shot at writing a valid deferred constraint?

A trivial example is CHECK (c IS NOT NULL) which, according to the
standard, is the only way to check for such a condition. The NOT NULL
syntax is explicitly translated to that by 11.4 <column definition> SR
17.a. We implement it a bit differently, but that does not negate the
usefulness of being able to defer it. In fact, all of the work Álvaro
is currently doing is mainly (or even fully) to be able to defer such a
constraint.

Is a deferred
constraint having those properties likely to be actually useful?

I believe the answer is yes.
--
Vik Fearing

#21Andreas Joseph Krogh
andreas@visena.com
In reply to: Dilip Kumar (#2)
#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Vik Fearing (#20)
#23David G. Johnston
david.g.johnston@gmail.com
In reply to: Andreas Joseph Krogh (#21)
#24Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#18)
#25Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#23)
#26David G. Johnston
david.g.johnston@gmail.com
In reply to: Robert Haas (#25)
#27Robert Haas
robertmhaas@gmail.com
In reply to: David G. Johnston (#26)
#28Vik Fearing
vik@postgresfriends.org
In reply to: Robert Haas (#27)