sql: CBO fails to run CHECK constraint on input row to UPSERT/INSERT ON CONFLICT

[knz]

The CBO runs CHECK only on the result of conflict resolution; it must run it on the input as well.

This is a regression (set optimizer=off provides the proper behavior)

> create table t(x int primary key, y int, constraint c check(y>10));
> insert into t(x,y) values (0,20);
>  insert into t(x,y) values (0,10) on conflict(x) do update set y= 20; -- fails in HP and PostgreSQL; succeeds with CBO

Related (but distinct from) #35364

cc @andy-kimball

Jira issue: CRDB-4590

[knz]

I think the CHECK constraint also needs to be run on the output, regardless; this should fail too given the example above:

> insert into t(x,y) values (0,30) on conflict(x) do update set y= 10;

[knz]

Discussed offline, @rmloveland this should be documented as a known limitation / difference with postgres, and we'll hope that users are OK with that.

We can leave the issue open (low priority) and perhaps label it with a new label "deliberate choice" so we can provide a better support experience when people ask.

[andy-kimball]

As another data point, @knz and I discovered that PG is inconsistent with applying CHECK constraint:

template1=# create table t(x int primary key, y int, constraint c check(y>10));
CREATE TABLE
template1=# insert into t(x,y) values (0,20) on conflict(x) do update set y=10;
INSERT 0 1
template1=# insert into t(x,y) values (0,10) on conflict(x) do update set y=20;
2019-03-04 14:52:34.541 PST [50218] ERROR:  new row for relation "t" violates check constraint "c"
2019-03-04 14:52:34.541 PST [50218] DETAIL:  Failing row contains (0, 10).
2019-03-04 14:52:34.541 PST [50218] STATEMENT:  insert into t(x,y) values (0,10) on conflict(x) do update set y=20;
ERROR:  new row for relation "t" violates check constraint "c"
DETAIL:  Failing row contains (0, 10).

Notice that PG does not check the constraint on set y=10 when the update is not made, but it does check the constraint on the insert values (0,10) even when the insert is not made.

I think the important thing for check constraints is to verify that bad data does not get inserted/updated into table. It doesn't seem too important for input that never makes it into the table to be checked. The fact that PG is sometimes checking input and sometimes not suggests their current behavior is just an implementation accident.

The CBO is consistent, in that it always checks the output values, and never checks the input values.

[jseldess]

@knz, can you help me with a blurb on this for the known limitations page?

[knz]

• pages/sections: UPSERT and INSERT ON CONFLICT
• Known limitation: "CockroachDB only validates CHECK constraints on the final result of a DML statement, to prevent rows that violate the constraint from being written in the table. This differs from PostgreSQL, which also validates CHECK constraints on the input rows of an UPSERT or INSERT ON CONFLICT statement, prior to conflict resolution."

[jseldess]

Documented in https://github.com/cockroachdb/docs/pull/5785.

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 5 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!

[knz]

This bug still exists in v21.1.

[github-actions[bot]]

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!