Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy editing constraint pages #964

Merged
merged 1 commit into from
Jan 24, 2017
Merged

copy editing constraint pages #964

merged 1 commit into from
Jan 24, 2017

Conversation

sploiselle
Copy link
Contributor

@sploiselle sploiselle commented Jan 5, 2017

  • Edited pages for constraints
  • Unified style for referencing constraints using format constraint. For example, "the Primary Key constraint". When talking about a concept and not a constraint itself, use the term without any caps. For example, "primary key".
  • Changed references of "Default value" to "Default Value."

Closes #313


This change is Reviewable

@dt
Copy link
Member

dt commented Jan 11, 2017

Not sure if we care, but lots of cases where we say things restrict values inserted but it constraints all writes, including UPDATES.

Mentioned in one place but applicable elsewhere: no difference between expressing a constraint on a col or at table level except for NULL and DEFAULT. We should probably document the table level and then only mention the single-col as a shorthand.


Reviewed 4 of 25 files at r1.
Review status: 4 of 25 files reviewed at latest revision, 11 unresolved discussions.


check.md, line 13 at r1 (raw file):

## Details

- If you add a Check constraint to an existing table, existing values are not checked. However, any updates to those values will be.

We might want to note that this is is a temporary limitation, not a permanent design decision -- in the future, the modifier NOT VALID would trigger this behavior (where the check is added quickly but only applies to new writes), but the default will be to check existing data. Currently we always do the NOT VALID behavior only because we haven't implemented queuing up the validation.


check.md, line 103 at r1 (raw file):

- [Constraints](constraints.html)
- [`DROP CONSTRAINTS`](drop-constraint.html)

Should this be plural?


constraints.md, line 7 at r1 (raw file):

---

Constraints provide additional data integrity beyond SQL's type checking. They let you define additional parameters for a column's data, which are checked whenever values are manipulated (inserted, deleted, or updated) and reject modifications that violate the constraint.

I don't know if I'd really think about them as related to type checking -- it's more about domain/business logic specific to your application being additionally enforced by the DB -- you could (and probably should, to have more control over error handling) check these things in your application, but also adding constraints on the db itself is an additional layer of safety, particularly for something like FKs where the app may operate on related tables separately.


constraints.md, line 9 at r1 (raw file):

Constraints provide additional data integrity beyond SQL's type checking. They let you define additional parameters for a column's data, which are checked whenever values are manipulated (inserted, deleted, or updated) and reject modifications that violate the constraint.

For example, the Unique constraint requires that all values in a column be unique from one another (except *NULL* values). If you attempt to insert a duplicate value, the constraint rejects the entire statement.

updates can also create dupes.


not-null.md, line 23 at r1 (raw file):

  CockroachDB tries to write a *NULL* value into column `b`. If that column has the Not Null constraint, the `INSERT` statement is rejected.

- You can only define the Not Null constraint when [creating a table](#syntax); you cannot add it to an existing table. However, you can [migrate data](constraints.html#table-migrations-to-add-or-change-immutable-constraints) from your current table to a new table with the constraint you want to use.

might want to note this is only current limitation, not a long-term decision.


not-null.md, line 75 at r1 (raw file):

- [Constraints](constraints.html)
- [`DROP CONSTRAINTS`](drop-constraint.html)

plural?


unique.md, line 3 at r1 (raw file):

---
title: Unique Constraint
summary: The Unique constraint specifies that each value in the constrained column must be unique (though *NULL* values are still allowed).

"still" seems strange to me (ditto below).
maybe just "non-null values must be unique?


unique.md, line 13 at r1 (raw file):

## Details

- You can still insert *NULL* values into columns with the Unique constraint because *NULL*s are not considered equal to other *NULL*s and therefore don't appear as the same value. This means that it's possible to insert rows that appear to be duplicates if one of the values is *NULL*.

I might lead with the fact that NULL is the absence of a value, so it is never equal and thus not considered a duplicate value. Thus if one uses NULL for one of the columns in a constraint etc.


unique.md, line 23 at r1 (raw file):

## Syntax

The syntax of the Unique constraint depends on whether you want the constraint to use a [single column](#single-column-column-level) or [multiple columns](#multiple-column-table-level).

Eh, you can use the table-level constraint on a single col -- in fact, except for DEFAULT and NULL, the col-level one is just a shorthand that we translate to table level constraint (on a single-col) before evaluating it, so that's what you'll see in SHOW CREATE TABLE.


unique.md, line 73 at r1 (raw file):

## Usage Example

Be aware that if a table has a Unique constraint on columns without the [Not Null constraint](not-null.html), it is possible to insert rows that appear to be duplicates if they contain a *NULL* value in at least one of the columns. This is because *NULL*s are never considered equal and hence don't violate the uniqueness constraint.

Maybe just me, but I found this one a little hard to follow / it sounded like we had some sort of defect that meant I needed to use NOT NULL for it to work, and it's duplicative with the statement above about nulls.


unique.md, line 101 at r1 (raw file):

- [Constraints](constraints.html)
- [`DROP CONSTRAINTS`](drop-constraint.html)

plural?


Comments from Reviewable

@jseldess
Copy link
Contributor

jseldess commented Jan 12, 2017

@sploiselle, would you mind incorporating this change into the foreign key doc as well? #990

Here's how Ben summarized it in this week's release notes:

The ALTER TABLE VALIDATE CONSTRAINT command can now be used to validate foreign keys in the UNVALIDATED state.

It's another ALTER TABLE substatement, though, so if you prefer we handle this separately, that's fine, too. Just seems like we'll need to reference the substatement in the FK doc.

@sploiselle
Copy link
Contributor Author

Review status: 4 of 25 files reviewed at latest revision, 11 unresolved discussions.


check.md, line 13 at r1 (raw file):

Previously, dt (David Taylor) wrote…

We might want to note that this is is a temporary limitation, not a permanent design decision -- in the future, the modifier NOT VALID would trigger this behavior (where the check is added quickly but only applies to new writes), but the default will be to check existing data. Currently we always do the NOT VALID behavior only because we haven't implemented queuing up the validation.

Done.


check.md, line 103 at r1 (raw file):

Previously, dt (David Taylor) wrote…

Should this be plural?

Done.


constraints.md, line 7 at r1 (raw file):

Previously, dt (David Taylor) wrote…

I don't know if I'd really think about them as related to type checking -- it's more about domain/business logic specific to your application being additionally enforced by the DB -- you could (and probably should, to have more control over error handling) check these things in your application, but also adding constraints on the db itself is an additional layer of safety, particularly for something like FKs where the app may operate on related tables separately.

Done.


constraints.md, line 9 at r1 (raw file):

Previously, dt (David Taylor) wrote…

updates can also create dupes.

Done.


not-null.md, line 23 at r1 (raw file):

Previously, dt (David Taylor) wrote…

might want to note this is only current limitation, not a long-term decision.

Done.


not-null.md, line 75 at r1 (raw file):

Previously, dt (David Taylor) wrote…

plural?

Done.


unique.md, line 3 at r1 (raw file):

Previously, dt (David Taylor) wrote…

"still" seems strange to me (ditto below).
maybe just "non-null values must be unique?

Done.


unique.md, line 13 at r1 (raw file):

Previously, dt (David Taylor) wrote…

I might lead with the fact that NULL is the absence of a value, so it is never equal and thus not considered a duplicate value. Thus if one uses NULL for one of the columns in a constraint etc.

Done.


unique.md, line 23 at r1 (raw file):

Previously, dt (David Taylor) wrote…

Eh, you can use the table-level constraint on a single col -- in fact, except for DEFAULT and NULL, the col-level one is just a shorthand that we translate to table level constraint (on a single-col) before evaluating it, so that's what you'll see in SHOW CREATE TABLE.

Done.


unique.md, line 73 at r1 (raw file):

Previously, dt (David Taylor) wrote…

Maybe just me, but I found this one a little hard to follow / it sounded like we had some sort of defect that meant I needed to use NOT NULL for it to work, and it's duplicative with the statement above about nulls.

Done.


unique.md, line 101 at r1 (raw file):

Previously, dt (David Taylor) wrote…

plural?

Done.


Comments from Reviewable

@sploiselle
Copy link
Contributor Author

@jseldess Just talked to @dt about VALIDATE CONSTRAINT and I'll open up a new PR with that stuff; has some substantial changes to the ADD CONSTRAINT doc.


Review status: 4 of 25 files reviewed at latest revision, 11 unresolved discussions.


Comments from Reviewable

@dt
Copy link
Member

dt commented Jan 12, 2017

:lgtm:


Review status: 3 of 25 files reviewed at latest revision, 11 unresolved discussions.


Comments from Reviewable

Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @sploiselle. Sorry it took me so long to review. Aside from my scattered comments, LGTM overall.

toc: false
---

Constraints offer additional data integrity by enforcing conditions on the data within a column or row. They are checked during DML operations and restrict the data values within a column to those specified within the constraint.
Constraints provide additional data integrity beyond SQL's type checking. They let you define additional parameters for a column's data, which are checked whenever values are manipulated (inserted, deleted, or updated) and reject modifications that violate the constraint.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"and reject modifications that violate the constraint" is off a bit, grammatically. I think it's probably fine to just end with "checked whenever values are manipulated(...)."

- You can have multiple Check constraints on a single column but ideally, for performance optimization, these should be combined using the logical operators. So, 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: column- or table-level


~~~ sql
warranty_period INT CHECK (warranty_period >= 0) CHECK (warranty_period <= 24)
~~~

should be specified as
should be specified as:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I liked this better without a colon, probably because we introduce the example with a colon already.

@@ -63,7 +66,7 @@ The syntax of the Check constraint depends on whether you want the constraint to
| `column_def` | Definitions for any other columns in the table. |
| `name` | The name you want to use for the constraint, which must be unique to its table and follow these [identifier rules](keywords-and-identifiers.html#identifiers). |
| `check_expr` | An expression that returns a Boolean value; if the expression evaluates to `FALSE`, the value cannot be inserted.|
| `table_constraints` | Any table-level [constraints](constraints.html) you want to apply. |
| `table_constraints` | Any other table-level [constraints](constraints.html) you want to apply. |

**Example**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: need space between table name and parenthesis:

CREATE TABLE inventory (

@@ -79,7 +82,7 @@ The syntax of the Check constraint depends on whether you want the constraint to

## Usage Example

Check constraints may be specified at the column or table level and can reference other columns within the table. Internally, all column level Check constrints are converted to table level constraints so they can be handled in a consistent fashion.
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 in a consistent fashion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same nit as above: column- or table-level

@@ -27,14 +27,14 @@ For example, if you create a foreign key on `orders.customer` that references `c
- Foreign key columns must be [indexed](indexes.html) when the table is created. To meet this requirement, there are a few options:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: In the html version, there seems to be extra whitespace between these two bullets, but I can't see anything that would cause that...

@@ -27,14 +27,14 @@ For example, if you create a foreign key on `orders.customer` that references `c
- Foreign key columns must be [indexed](indexes.html) when the table is created. To meet this requirement, there are a few options:

- Create indexes explicitly using the [`INDEX`](create-table.html#create-a-table-with-secondary-indexes) clause of `CREATE TABLE`.
- Rely on indexes created by [`PRIMARY KEY`](primary-key.html) or [`UNIQUE`](unique.html) constraints.
- Rely on indexes created by the [Primary Key](primary-key.html) or [Unique](unique.html) constraints.
- Have CockroachDB automatically create an index of the foreign key columns for you. However, it's important to note that if you later remove the Foreign Key constraint, this automatically created index _is not_ removed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to capitalize Foreign Key before columns, or just before constraints? If the former, there are few instances for you to find a replace.

{% include sql/diagrams/not_null_column_level.html %}

| Parameter | Description |
|-----------|-------------|
| `table_name` | The name of the table you're creating. |
| `column_name` | The name of the constrained column. |
| `column_type` | The constrained column's [data type](data-types.html). |
| `column_constraints` | Any other column-level [constraints](constraints.html) you want to apply. |
| `column_constraints` | Any other column-level [constraints](constraints.html) you want to apply to this column. |
| `column_def` | Definitions for any other columns in the table. |
| `table_constraints` | Any table-level [constraints](constraints.html) you want to apply. |

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we need this example, since it's identical to the start of the Usage Example below.

{{site.data.alerts.callout_info}}Strictly speaking, a primary key's unique index is not created, it's derived from the keys under which the data is stored so it takes no additional space. However, it appears as a normal unique index when using commands like <code>SHOW INDEXES</code>.{{site.data.alerts.end}}
- Columns that are part of a Primary Key are mandatory (NOT NULL). If an optional (nullable) column is made part of a Primary Key, it is made mandatory (NOT NULL).
- Tables can only have one primary key.
- To ensure each row has a unique identifier, the Primary Key constraint combines the properties of both the [Unique](unique.html) and [Not Null](not-null.html) constraints. The properties of both constraints are necessary to make sure each row's primary key columns contain sets of values that do not appear to be the same.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: instead of "values that do not appear to be the same", can't we just say, "values that are not the same"?

If you need to strictly enforce uniqueness, use the [Not Null constraint](not-null.html) in addition to the Unique constraint. You can also achieve the same behavior through the table's [Primary Key](primary-key.html).

- Columns with the Unique constraint automatically have an [index](indexes.html) created with the name `<table name>_<columns>_key`. To avoid having two identical indexes, you should not create indexes that exactly match the Unique constraint's columns and order. <br/><br/>The Unique constraint depends on the automatically created index, so dropping the index also drops the Unique constraint.
- When using the Unique constraint on multiple columns, the collective values of the columns must be unique. This *does not* mean that each value in each column must Unique, as if you had applied the Unique constraint to each column individually.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must Unique --> must be Unique

@jseldess
Copy link
Contributor

Oh, one additional request: in the sidenav, let's change Foreign Keys to Foreign Key to match the form of the other constraints.

@sploiselle sploiselle force-pushed the copy-edit-constraints branch 3 times, most recently from 35ece1d to 22c6f41 Compare January 24, 2017 15:18
@jseldess
Copy link
Contributor

:lgtm:


Reviewed 17 of 25 files at r1, 1 of 7 files at r2, 8 of 8 files at r3.
Review status: all files reviewed at latest revision, 30 unresolved discussions.


Comments from Reviewable

@sploiselle sploiselle merged commit 40a4186 into gh-pages Jan 24, 2017
@sploiselle sploiselle deleted the copy-edit-constraints branch January 24, 2017 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants