drop CHECK when column is dropped #2638

lnhsingh · 2018-03-08T17:43:45Z

Closes #2391.

cockroach-teamcity · 2018-03-08T17:43:49Z

This change is

cockroach-teamcity · 2018-03-08T17:45:31Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/560ed8537551fec4ff943259486927fbbeb3b587/

Closes #2391.

cockroach-teamcity · 2018-03-08T20:07:35Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/d62345ad89ad19b3efb5a0c9bbbf742d7ee125bf/

jseldess

LGTM, with a few nites.

You should also get @knz to review, since he made this change in the product.

jseldess · 2018-03-08T21:06:39Z

v2.0/check.md

+- If you add a `CHECK` constraint to an existing table, existing values are not checked. However, any updates to those values will be.
+  {{site.data.alerts.callout_info}}In the future we plan to expand the `CHECK` constraint to include a check on any existing values in the column.{{site.data.alerts.end}}
+- `CHECK` constraints may be specified at the column or table level and can reference other columns within the table. Internally, all column-level Check constraints are converted to table-level constraints so they can be handled consistently.
+- You can have multiple `CHECK` constraints on a single column but ideally, for performance optimization, these should be combined using the logical operators. For example


nit: For example:

jseldess · 2018-03-08T21:07:46Z

v2.0/check.md

  ~~~ sql
  warranty_period INT CHECK (warranty_period BETWEEN 0 AND 24)
  ~~~
+- When a column with a `CHECK` constraint is dropped, the `CHECK` constaint is also dropped.


spelling: constaint > constraint

jseldess · 2018-03-08T21:08:34Z

v2.0/drop-column.md

@@ -21,7 +21,7 @@ The user must have the `CREATE` [privilege](privileges.html) on the table.
 | Parameter | Description |
 |-----------|-------------|
 | `table_name` | The name of the table with the column you want to drop. |
-| `name` | The name of the column you want to drop. |
+| `name` | The name of the column you want to drop.<br><br>When a column with a `CHECK` constraint is dropped, the `CHECK` constaint is also dropped. |


Same as above.

cockroach-teamcity · 2018-03-08T22:36:29Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/b39ca1d129c3dada0cf0844b1f496743635772db/

knz · 2018-03-08T22:53:12Z

v2.0/check.md

- Check constraints may be specified at the column or table level and can reference other columns within the table. Internally, all column-level Check constraints are converted to table-level constraints so they can be handled consistently.
- You can have multiple Check constraints on a single column but ideally, for performance optimization, these should be combined using the logical operators. For example
+- If you add a `CHECK` constraint to an existing table, existing values are not checked. However, any updates to those values will be.
+  {{site.data.alerts.callout_info}}In the future we plan to expand the `CHECK` constraint to include a check on any existing values in the column.{{site.data.alerts.end}}


This should link to the github issue where progress is tracked.

I found #8773 but can't find the specific issue for CHECK constraint. Do you know what issue is used to track this?

cockroachdb/cockroach#23663

👍 thanks!

knz · 2018-03-08T22:53:27Z

v2.0/check.md

- You can have multiple Check constraints on a single column but ideally, for performance optimization, these should be combined using the logical operators. For example
+- If you add a `CHECK` constraint to an existing table, existing values are not checked. However, any updates to those values will be.
+  {{site.data.alerts.callout_info}}In the future we plan to expand the `CHECK` constraint to include a check on any existing values in the column.{{site.data.alerts.end}}
+- `CHECK` constraints may be specified at the column or table level and can reference other columns within the table. Internally, all column-level Check constraints are converted to table-level constraints so they can be handled consistently.


all column-level Check constraints -> all column-level CHECK constraints

cockroach-teamcity · 2018-03-09T16:22:20Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/4cfdf9255a494b83e03f5313453fb4411ce6a306/

knz · 2018-03-09T16:39:39Z

v2.0/check.md

- Check constraints may be specified at the column or table level and can reference other columns within the table. Internally, all column-level Check constraints are converted to table-level constraints so they can be handled consistently.
- You can have multiple Check constraints on a single column but ideally, for performance optimization, these should be combined using the logical operators. For example
+- If you add a `CHECK` constraint to an existing table, existing values are not checked. However, any updates to those values will be.
+  {{site.data.alerts.callout_info}}In the future we plan to [expand the `CHECK` constraint](https://github.com/cockroachdb/cockroach/issues/23663) to include a check on any existing values in the column.{{site.data.alerts.end}}


@dt just pointed out this point (and the callout) are plainly wrong. This limitation was lifted in cockroachdb 1.1.
(in cockroachdb/cockroach#9152 )
One needs to use VALIDATE to check the previous rows.

Ok, thanks for clarifying. I updated this bullet point for 2.0 and 1.1

cockroach-teamcity · 2018-03-09T17:44:51Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/fb8fbfe716a4c13cab60670710882bc1ce261583/

knz · 2018-03-09T17:56:46Z

Thanks, LGTM

lnhsingh requested a review from jseldess March 8, 2018 17:43

lnhsingh added the in progress label Mar 8, 2018

drop CHECK when column is dropped

d62345a

Closes #2391.

lnhsingh force-pushed the fixit-check branch from 560ed85 to d62345a Compare March 8, 2018 20:06

jseldess approved these changes Mar 8, 2018

View reviewed changes

minor spelling edits

b39ca1d

lnhsingh requested a review from knz March 8, 2018 22:35

knz reviewed Mar 8, 2018

View reviewed changes

Lauren added 2 commits March 9, 2018 10:53

minor style edit

674f9d6

add github link

4cfdf92

knz reviewed Mar 9, 2018

View reviewed changes

clarify bullet point about checking existing values

fb8fbfe

jseldess merged commit 4d070ef into master Mar 9, 2018

jseldess deleted the fixit-check branch March 9, 2018 18:33

jseldess removed the in progress label Mar 9, 2018

lnhsingh added the fixitday label Mar 9, 2018

drop CHECK when column is dropped #2638

drop CHECK when column is dropped #2638

Uh oh!

Conversation

lnhsingh commented Mar 8, 2018

Uh oh!

cockroach-teamcity commented Mar 8, 2018

Uh oh!

cockroach-teamcity commented Mar 8, 2018

Uh oh!

cockroach-teamcity commented Mar 8, 2018

Uh oh!

jseldess left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cockroach-teamcity commented Mar 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cockroach-teamcity commented Mar 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cockroach-teamcity commented Mar 9, 2018

Uh oh!

knz commented Mar 9, 2018

Uh oh!

Uh oh!