Horton

Thread

  1. CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-07-05T09:37:44Z

    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

  2. Re: CHECK Constraint Deferrable

    Dilip Kumar <dilipbalaut@gmail.com> — 2023-07-07T11:50:44Z

    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

  3. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-07-07T14:00:10Z

    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

  4. Re: CHECK Constraint Deferrable

    David G. Johnston <david.g.johnston@gmail.com> — 2023-07-07T14:04:41Z

    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.

  5. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-09-07T07:55:05Z

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

Thanks,
Himanshu

  6. Re: CHECK Constraint Deferrable

    Dilip Kumar <dilipbalaut@gmail.com> — 2023-09-08T07:52:42Z

    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

  7. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-09-11T12:15:14Z

    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

  8. Re: CHECK Constraint Deferrable

    vignesh C <vignesh21@gmail.com> — 2023-09-12T09:25:48Z

    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]

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

  9. Re: CHECK Constraint Deferrable

    vignesh C <vignesh21@gmail.com> — 2023-09-14T04:27:15Z

    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

  10. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-09-14T06:14:34Z

    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

  11. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-09-14T10:03:19Z

    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

  12. Re: CHECK Constraint Deferrable

    vignesh C <vignesh21@gmail.com> — 2023-09-15T05:58:47Z

    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

  13. Re: CHECK Constraint Deferrable

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2023-09-19T10:44:04Z

    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

  14. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-10-02T15:01:17Z

    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

  15. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-10-02T15:01:22Z

    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

  16. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-10-02T15:01:34Z

    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

  17. Re: CHECK Constraint Deferrable

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2023-10-02T15:03:11Z

    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

  18. Re: CHECK Constraint Deferrable

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-02T19:25:10Z

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

  19. Re: CHECK Constraint Deferrable

    David G. Johnston <david.g.johnston@gmail.com> — 2023-10-02T20:16:41Z

    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.

  20. Re: CHECK Constraint Deferrable

    Vik Fearing <vik@postgresfriends.org> — 2023-10-02T22:56:20Z

    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:
https://www.postgresql.org/message-id/flat/12539.1544107316%40sss.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

  21. Re: CHECK Constraint Deferrable

    Andreas Joseph Krogh <andreas@visena.com> — 2023-10-03T00:06:00Z

    
På fredag 07. juli 2023 kl. 13:50:44, skrev Dilip Kumar <dilipbalaut@gmail.com 
<mailto:dilipbalaut@gmail.com>>:
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.


The real-world use case, at least for me, is when using an ORM. For large 
object-graphs ORMs have a tendency to INSERT first with NULLs then UPDATE the 
“NOT NULLs” later.

“Rewrite the ORM” is not an option for most of us…



--

Andreas Joseph Krogh

  22. Re: CHECK Constraint Deferrable

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-03T00:08:37Z

    Vik Fearing <vik@postgresfriends.org> writes:
> On 10/2/23 21:25, Tom Lane wrote:
>> Sorry for not weighing in on this before, but ... is this a feature
>> we want at all?

> For standards conformance, I vote yes.

Only if we can actually implement it in a defensible way, which this
patch is far short of accomplishing.

>> We are very clear in the existing docs that CHECK
>> conditions must be immutable [1],

> 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:
> https://www.postgresql.org/message-id/flat/12539.1544107316%40sss.pgh.pa.us

What I intended to defend was not *checking* immutability strictly.
Our CHECK constraint implementation is based very much on the assumption
that the constraints are immutable, and nobody has proposed that we
try to remove that assumption AFAIR.  So I think the docs are fine
as-is; anybody who wants to get into monotonically-weakening constraints
is probably smart enough to work out for themselves whether it will
fly or not.

So my problem with this patch is that it does nothing about that
assumption, and yet the feature it adds seems useless without
weakening the assumption.  So what weaker assumption could we
make, and how would we modify the when-to-check rules to match
that, and what would it cost us in performance?  Without good
answers to those questions, this patch is just a facade.

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

What cleanup would you need that couldn't be performed beforehand
(e.g. in a BEFORE INSERT/UPDATE trigger)?  All the practical
examples that occur to me involve cross-row conditions, which
CHECK is unsuitable to enforce --- at least, without doing a
thorough implementation rethink.

I continue to assert that basing this feature on the current
CHECK implementation will produce nothing but a toy feature,
that's not only of little practical use but will be an active
foot-gun for people who expect it to do more than it can.

			regards, tom lane

  23. Re: CHECK Constraint Deferrable

    David G. Johnston <david.g.johnston@gmail.com> — 2023-10-03T13:53:35Z

    On Monday, October 2, 2023, Andreas Joseph Krogh <andreas@visena.com> wrote:

> På fredag 07. juli 2023 kl. 13:50:44, skrev Dilip Kumar <
> dilipbalaut@gmail.com>:
>
> 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.
>
>
>
> The real-world use case, at least for me, is when using an ORM. For large
> object-graphs ORMs have a tendency to INSERT first with NULLs then UPDATE
> the “NOT NULLs” later.
>
> “Rewrite the ORM” is not an option for most of us…
>
Between this and Vik comment it sounds like we should probably require a
patch in this area to solve both the not null and check constraint deferral
omissions then, not just one of them (alternatively, let’s solve the not
null one first).

David J.

  24. Re: CHECK Constraint Deferrable

    Robert Haas <robertmhaas@gmail.com> — 2023-10-09T20:11:18Z

    On Mon, Oct 2, 2023 at 10:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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'm a little bit confused by this whole discussion because surely this
statement is just completely false. The example in the original post
demonstrates that clearly.

The use case for a deferred check constraint is exactly the same as
the use case for a deferred foreign key constraint or a deferred
uniqueness constraint, which is that you might have a constraint that
will be temporarily false while the transaction is in progress, but
true by the time the transaction actually commits, and you might like
the transaction to succeed instead of failing in such a case. You seem
to be imagining that the constraint itself might be returning mutable
answers on the same inputs, but that's not what this is about at all.

I'm not here - at least, not right now - to take a position on whether
the patch itself is any good.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  25. Re: CHECK Constraint Deferrable

    Robert Haas <robertmhaas@gmail.com> — 2023-10-09T20:26:52Z

    On Tue, Oct 3, 2023 at 10:05 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>> The real-world use case, at least for me, is when using an ORM. For large object-graphs ORMs have a tendency to INSERT first with NULLs then UPDATE the “NOT NULLs” later.
>>
>> “Rewrite the ORM” is not an option for most of us…
>
> Between this and Vik comment it sounds like we should probably require a patch in this area to solve both the not null and check constraint deferral omissions then, not just one of them (alternatively, let’s solve the not null one first).

I have a couple of problems with this comment:

1. I don't know which of Vik's comments you're talking about here, but
it seems to me that Vik is generally in favor of this feature, so I'm
a bit surprised to hear that one of his comments led you to think that
it should be burdened with additional requirements.

2. I don't think it's a good idea for the same patch to try to solve
two problems unless they are so closely related that solving one
without solving the other is not sensible. It is a good policy for the
community to accept incremental progress provided it doesn't break
things along the way. Smaller patches are way easier to get committed,
and then we get some of the feature sooner instead of all of it some
more distant point in the future or never. Furthemore, forcing
additional requirements onto patch submitters as a condition of patch
acceptance is extremely demoralizing to submitters, and we should not
do it without an excellent reason.

Mind you, I'm not against this patch handling both CHECK and NOT NULL
constraints if that's the most sensible way forward, especially in
view of Álvaro's recent work in that area. But it sort of sounds like
you're just trying to sink the patch despite it being a feature that
is both in the SQL standard and has real use cases which have been
mentioned on the thread, and I don't really like that. In the interest
of full disclosure, I do work at the same company as Dilip and
Himanshu, so I might be biased. But I feel like I would be in favor of
this feature no matter who proposed it, as long as it was
well-implemented. It's always struck me as odd that we allow deferring
some types of constraints but not others, and I don't understand why
we'd want to block closing that gap unless there is some concrete
downside to so doing.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  26. Re: CHECK Constraint Deferrable

    David G. Johnston <david.g.johnston@gmail.com> — 2023-10-09T21:07:35Z

    On Mon, Oct 9, 2023 at 1:27 PM Robert Haas <robertmhaas@gmail.com> wrote:

> On Tue, Oct 3, 2023 at 10:05 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> >> The real-world use case, at least for me, is when using an ORM. For
> large object-graphs ORMs have a tendency to INSERT first with NULLs then
> UPDATE the “NOT NULLs” later.
> >>
> >> “Rewrite the ORM” is not an option for most of us…
> >
> > Between this and Vik comment it sounds like we should probably require a
> patch in this area to solve both the not null and check constraint deferral
> omissions then, not just one of them (alternatively, let’s solve the not
> null one first).
>
> I have a couple of problems with this comment:
>
> 1. I don't know which of Vik's comments you're talking about here, but
> it seems to me that Vik is generally in favor of this feature, so I'm
> a bit surprised to hear that one of his comments led you to think that
> it should be burdened with additional requirements.
>

Specifically, Vik commented that the standard requires implementing NOT
NULL as a check constraint and thus needs to allow for deferrable check
constraints in order for not null checks to be deferrable.  I agree fully
that deferring a not null check makes for an excellent use case.  But we've
also decided to make NOT NULL its own thing, contrary to the standard.
Thus my understanding for why this behavior is standard mandated is that it
is to allow for deferrable not null constraints and thus our goal should be
to make our not null constraints deferrable.

The only other example case of wanting a deferrable check constraint
involved the usage of a function that we expressly prohibit as a check
constraint.  The argument, which I weakly support, is that if our adding
deferrable check constraints increases the frequency of such functions
being created and used by our users, then we should continue to prohibit
such deferrability and require those users to properly implement triggers
which can then be deferred.  With a deferrable not null constraint any
other reasonable check constraints can simply evaluate to null during the
period where they should be deferred - because their column inputs are
deferred nulls - and then we be fully evaluated when the inputs ultimately
end up non-null.

2. I don't think it's a good idea for the same patch to try to solve
> two problems unless they are so closely related that solving one
> without solving the other is not sensible.


A NOT NULL constraint apparently is just a special case of a check
constraint which seems closely related enough to match your definition.

But I guess you are right, I was trying to say no to this patch, and yes to
the not null deferral idea, without being so explicit and final about it.

While the coders are welcome to work on whatever they wish, the effort
spent on this just doesn't seem that valuable compared to what is already
in the queue being worked on needing reviews and commits. I can live with a
gap in our standards conformance here since I haven't observed any uses
cases that are impossible to accomplish except by adding this specific
feature which would only cover NOT NULL constraints if the syntactical form
for creating them were not used (which I suppose if we fail to provide NOT
NULL DEFERRABLE that would argue for at least giving this work-around...)

David J.

  27. Re: CHECK Constraint Deferrable

    Robert Haas <robertmhaas@gmail.com> — 2023-10-10T14:12:36Z

    On Mon, Oct 9, 2023 at 5:07 PM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>> 2. I don't think it's a good idea for the same patch to try to solve
>> two problems unless they are so closely related that solving one
>> without solving the other is not sensible.
>
> A NOT NULL constraint apparently is just a special case of a check constraint which seems closely related enough to match your definition.

Yes, that might be true. I suppose I'd like to hear from the patch
author(s) about that. I'm somewhat coming around to your idea that
maybe both should be covered together, but I'm not the one writing the
patch.

> But I guess you are right, I was trying to say no to this patch, and yes to the not null deferral idea, without being so explicit and final about it.

But this, I dislike, for reasons which I'm sure you can appreciate. As
you say, people are free to choose their own development priorities. I
don't need this feature for anything either, personally, but my need
or lack of it for some particular feature doesn't define the objective
usefulness thereof. And to be honest, if I were trying to step back
from my personal needs, I'd say this seems likely to be more useful
than 75% of what's in the CommitFest. Your judgement can be different
and that's fine too, but I think the argument for calling this useless
is weak, especially given that several people have already mentioned
ways that they would like to use it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  28. Re: CHECK Constraint Deferrable

    Vik Fearing <vik@postgresfriends.org> — 2023-10-13T00:36:02Z

    On 10/10/23 15:12, Robert Haas wrote:
> On Mon, Oct 9, 2023 at 5:07 PM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
>>> 2. I don't think it's a good idea for the same patch to try to solve
>>> two problems unless they are so closely related that solving one
>>> without solving the other is not sensible.
>>
>> A NOT NULL constraint apparently is just a special case of a check constraint which seems closely related enough to match your definition.
> 
> Yes, that might be true. I suppose I'd like to hear from the patch
> author(s) about that. I'm somewhat coming around to your idea that
> maybe both should be covered together, but I'm not the one writing the
> patch.

Álvaro Herrera has put (and is still putting) immense effort into 
turning NOT NULL into a CHECK constraint.

Honestly, I don't see why the two patches need to be combined.
-- 
Vik Fearing