sql: Check CHECK constraints on upsert when updating a conflicting row #26642
Conversation
|
This change is |
|
Very nice. Pleasantly suprised how little time this took you. Congratulations. Very good commit message too. :) Some more tests needed, see below. Reviewed 2 of 2 files at r1. pkg/sql/logictest/testdata/logic_test/upsert, line 546 at r1 (raw file):
Right now your code is lucky that the insert column order is the same as the table column order. So this does not verify that column ordering is handled properly.
Comments from Reviewable |
complete! 0 of 0 LGTMs obtained
|
Another thing: when you create a PR which, when merges, causes an existing issue to be resolved, write "Fixes #NNN." somewhere in the PR description. This will make github do the right thing automatically when the PR merges. Review status: Comments from Reviewable |
|
are "closes" and "fixes" functionally equivalent? Review status: pkg/sql/logictest/testdata/logic_test/upsert, line 546 at r1 (raw file): Previously, knz (kena) wrote…
Done Comments from Reviewable |
Yep. Comments from Reviewable |
|
As usual you'll need to squash this. This fix makes me happy, this was a high severity bug. Also because this was a high severity bug, after this merges you'll need to back-port your fix to our 2.0 release branch. Please refer to the wiki for details (I think it's documented here). Do not forget about Nikhil's Reviewed 1 of 1 files at r2. Comments from Reviewable |
* Before: On a table with a CHECK constraint, such as:
```
CREATE TABLE ex(foo INT PRIMARY KEY, bar INT, CHECK (bar < 2)
```
an upsert on the table would not properly check the CHECK constraints,
e.g.
```
INSERT INTO ex(foo, bar) VALUES(1, 2);
INSERT INTO ex(foo, bar) VALUES(1, 2) ON CONFLICT (foo) DO UPDATE SET
bar = 3
```
would update the row, violating the check constraint.
* After: On attempting such an operation, CockroachDB now throws the
proper database error "failed to satisfy CHECK constraint ..."
Release note (sql change): CHECK constraints are now checked when
updating a conflicting row in INSERT ... ON CONFLICT DO UPDATE
statements.
emsal0
force-pushed
the
upsert-on-conflict-check-constraint
branch
from
a478f39 to
8bd51b6
Compare
|
bors r+ |
26642: sql: Check CHECK constraints on upsert when updating a conflicting row r=emsal1863 a=emsal1863
* Before: On a table with a CHECK constraint, such as:
```
CREATE TABLE ex(foo INT PRIMARY KEY, bar INT, CHECK (bar < 2)
```
an upsert on the table would not properly check the CHECK constraints,
e.g.
```
INSERT INTO ex(foo, bar) VALUES(1, 2);
INSERT INTO ex(foo, bar) VALUES(1, 2) ON CONFLICT (foo) DO UPDATE SET
bar = 3
```
would update the row, violating the check constraint.
* After: On attempting such an operation, CockroachDB now throws the
proper database error "failed to satisfy CHECK constraint ..."
Release note (sql change): CHECK constraints are now checked when
updating a conflicting row in INSERT ... ON CONFLICT DO UPDATE
statements.
closes #23699
Co-authored-by: Emmanuel <[email protected]>
Build succeeded |
26699: release-2.0: sql: Check CHECK constraints on upsert when updating a conflicting row r=emsal1863 a=emsal1863 Backport 1/1 commits from #26642. /cc @cockroachdb/release --- * Before: On a table with a CHECK constraint, such as: ``` CREATE TABLE ex(foo INT PRIMARY KEY, bar INT, CHECK (bar < 2) ``` an upsert on the table would not properly check the CHECK constraints, e.g. ``` INSERT INTO ex(foo, bar) VALUES(1, 2); INSERT INTO ex(foo, bar) VALUES(1, 2) ON CONFLICT (foo) DO UPDATE SET bar = 3 ``` would update the row, violating the check constraint. * After: On attempting such an operation, CockroachDB now throws the proper database error "failed to satisfy CHECK constraint ..." Release note (sql change): CHECK constraints are now checked when updating a conflicting row in INSERT ... ON CONFLICT DO UPDATE statements. closes #23699 Co-authored-by: Emmanuel <[email protected]>
Before: On a table with a CHECK constraint, such as:
an upsert on the table would not properly check the CHECK constraints,
e.g.
would update the row, violating the check constraint.
After: On attempting such an operation, CockroachDB now throws the
proper database error "failed to satisfy CHECK constraint ..."
Release note (sql change): CHECK constraints are now checked when
updating a conflicting row in INSERT ... ON CONFLICT DO UPDATE
statements.
closes #23699