Обсуждение: 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

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. 

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

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

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

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

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

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

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

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

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…

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

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
INSERTfirst 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
nulland 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

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
matchyour 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
soexplicit 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

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