Completed Update statement documentation #1707

Amruta-Ranade · 2017-07-13T03:04:40Z

Closes #499
Need to self-review the content, but fixed formatting issues.

cockroach-teamcity · 2017-07-13T03:04:44Z

This change is

cockroach-teamcity · 2017-07-13T03:08:23Z

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

jseldess

Glad you resolved that formatting issue, @Amruta-Ranade! Overall, this is looking good. I have a few comments. But I think it'd be best for you to get this reviewed by an engineer first, whoever you worked with (David?).

jseldess · 2017-07-13T10:46:48Z

v1.0/update.md

+`column_name` | The name of the column whose values you want to update. To update the values of multiple columns, use a comma-separated list of column names.
+`a_expr` | The new value you want to use, the [aggregate function](functions-and-operators.html#aggregate-functions) you want to perform, or the [value expression](sql-expressions.html) you want to use.
+`DEFAULT` | To fill columns with their [default values](default-value.html), use `DEFAULT VALUES` in place of `a_expr`. To fill a specific column with its default value, leave the value out of the `a_expr` or use `DEFAULT` at the appropriate position. 
+`column_name_list` | A comma-separated list of column names, in parentheses.


Since column_name is represented in the diagram as a loop, and you state that you can use a comma-separated list in the column_name description, I think it's redundant to list column_name_list here and in the diagram. What are you thoughts?

Actually, that's an error I made. In the first instance, it cannot be a comma-separated list of column names. It can be a comma-separated list of column name-value pairs. In the second instance, it can be a comma-separated list of column names. Fixed that in the document.

I'm still confused about the difference between these. What is a comma-separated list of column name-value pairs vs. a comma-separate list of column names? Can you provide an example of each to help me understand?

jseldess · 2017-07-13T10:47:40Z

v1.0/update.md

+`DEFAULT` | To fill columns with their [default values](default-value.html), use `DEFAULT VALUES` in place of `a_expr`. To fill a specific column with its default value, leave the value out of the `a_expr` or use `DEFAULT` at the appropriate position. 
+`column_name_list` | A comma-separated list of column names, in parentheses.
+`select_with_parens` | A comma-separated list of values or [value expressions](sql-expressions.html), in parentheses. To update values of multiple rows, use a comma-separated list of parentheses. <br><br>Each value must match the [data type](data-types.html) of its column. Also, if column names are listed (`qualified_name_list`), values must be in corresponding order; otherwise, they must follow the declared order of the columns in the table.
+`WHERE a_expr`| `a_expr` must be an expression that returns Boolean values using columns (e.g. `<column> = <value>`). Update rows that return `TRUE`.<br><br>__Without a `WHERE` clause in your statement, `UPDATE` updates all rows from the table.


How did you want to format the final paragraph? I think something's incomplete there...

Yes, that's an error that creeped in when I was debugging the formatting issue. Fixed in the document.

Looks like the last paragraph still starts with two underscores. What formatting are you intending here?

jseldess · 2017-07-13T10:52:11Z

v1.0/update.md

+| id | balance  | customer |
+----+----------+----------+
+|  1 | 10000.50 | michael  |
+|  2 |   9000.0 | kelly   |


Need one more space before the final pipe of row 2.

Still looks the same. Did you push your change?

jseldess · 2017-07-13T10:52:33Z

v1.0/update.md

+(4 rows)
+~~~
+
+### Update with default values


Use title case, like for the other examples: Update with Default Values

Still looks the same...

jseldess · 2017-07-13T10:54:58Z

v1.0/update.md

+
+### Update All Rows
+
+{{site.data.alerts.callout_danger}}If you don't use the WHERE clause to specify the rows to be updated, the values for all rows will be updated.{{site.data.alerts.end}}


Need to apply code formatting to WHERE. Note that within a callout like this, you need to use html to format things:

<code>WHERE</code>

Still looks the same...

jseldess · 2017-07-13T10:57:24Z

v1.0/update.md

+
+### Update and Return Values
+
+In this example, the `RETURNING` clause returns the `id` values of the rows updated, which are generated server-side by the `unique_rowid()` function. The language-specific versions assume that you have installed the relevant [client drivers](install-client-drivers.html).


Some of the text taken from the insert example is not correct here. You need to remove the clause:

which are generated server-side by the unique_rowsid() function.

Also, the first sentence needs to be revised to match the fact that the example updates a single row:

In this example, the RETURNING clause returns the id value of the row updated.

Still looks the same...

jseldess · 2017-07-13T10:58:07Z

v1.0/update.md

+
+In this example, the `RETURNING` clause returns the `id` values of the rows updated, which are generated server-side by the `unique_rowid()` function. The language-specific versions assume that you have installed the relevant [client drivers](install-client-drivers.html).
+
+{{site.data.alerts.callout_success}}This use of <code>RETURNING</code> mirrors the behavior of MySQL's <code>last_insert_id()</code> function.{{site.data.alerts.end}}


We should remove this as well. It applies more to the insert example from which this was taken.

I checked online, and it seems the last_insert_id() function works with update as well. Will confirm with eng though.

OK, thanks, Amruta.

jseldess · 2017-07-13T11:02:09Z

v1.0/update.md

+    'UPDATE accounts SET balance = DEFAULT WHERE id = 1 RETURNING id'
+)
+
+# Print out the returned values.


Make it singular to match the example: returned value.

Still looks the same. Seems like you haven't pushed your changes yet, for this and all other changes below?

jseldess · 2017-07-13T11:02:42Z

v1.0/update.md

+
+# Print out the returned values.
+rows = cur.fetchall()
+print('IDs:')


Again, make it singular to match the example: print('ID:')

jseldess · 2017-07-13T11:03:06Z

v1.0/update.md

+conn.close()
+~~~
+
+The printed values would look like:


Make it singular: The printed value would look like:

jseldess · 2017-07-13T11:03:24Z

v1.0/update.md

+The printed values would look like:
+
+~~~
+IDs:


Make it singular: ID:

jseldess · 2017-07-13T11:05:48Z

v1.0/update.md

+
+</section>
+
+<section class="filter-content" markdown="1" data-scope="ruby">


Same tweaks need to be made to this and all the other code samples.

cockroach-teamcity · 2017-07-14T00:26:07Z

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

cockroach-teamcity · 2017-07-14T00:29:00Z

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

sploiselle · 2017-07-14T01:55:34Z

Review status: 0 of 3 files reviewed at latest revision, 12 unresolved discussions, some commit checks failed.

v1.0/update.md, line 44 at r1 (raw file):

| id | balance  | customer |
+----+----------+----------+
|  1 | 10000.50 | michael  |

Looks like Jesse has gotten a lot of feedback on this, but I'd be really stoked if we could use non-WASP names! I recommend using a random name generator like this one: https://www.behindthename.com/random/

Comments from Reviewable

Amruta-Ranade · 2017-07-14T02:00:10Z

@sploiselle This is cool! Thanks for sharing!

cockroach-teamcity · 2017-07-14T16:39:52Z

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

cockroach-teamcity · 2017-07-17T16:51:21Z

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

…s, Added non-WASP names, regrouped examples

cockroach-teamcity · 2017-07-17T17:15:25Z

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

jseldess · 2017-07-17T21:08:53Z

Review status: 0 of 3 files reviewed at latest revision, 14 unresolved discussions, all commit checks successful.

v1.0/update.md, line 106 at r2 (raw file):

~~~

### Update using SELECT statement

Thanks for adding this, Amruta!

nit: Use title case like the others, and code-format SELECT:

Update Using `SELECT` Statement

v1.0/update.md, line 223 at r2 (raw file):

# Insert two rows into the "accounts" table
# and return the "id" value.

I meant to mention this earlier, but it looks like my comment got lost: We need to change this comment to:

# Update a row in the "accounts" table
# and return the "id" value.

Also need to update the other code samples.

Comments from Reviewable

…amples

cockroach-teamcity · 2017-07-17T21:17:31Z

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

jseldess · 2017-07-17T21:24:04Z

Thanks, @Amruta-Ranade. LGTM (after you apply these updates to the v1.1 page as well).

cockroach-teamcity · 2017-07-18T14:52:34Z

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

cockroach-teamcity · 2017-07-18T18:45:25Z

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

Amruta-Ranade requested a review from jseldess July 13, 2017 03:04

jseldess reviewed Jul 13, 2017

View reviewed changes

Completed Update statement documentation, Incorporated review comment…

5c8fc5d

…s, Added non-WASP names, regrouped examples

Amruta-Ranade force-pushed the update_stmt branch from 60435aa to 5c8fc5d Compare July 17, 2017 17:10

Amruta-Ranade requested a review from dt July 17, 2017 17:28

Amruta-Ranade changed the title ~~[WIP]: Completed Update statement documentation~~ Completed Update statement documentation Jul 17, 2017

Fixed title case, replaced instances of insert with update in code ex…

56c6744

…amples

Updated v1.1 update.md

bcd3fa1

dt approved these changes Jul 18, 2017

View reviewed changes

Amruta-Ranade merged commit 5f9abe8 into master Jul 18, 2017

Amruta-Ranade deleted the update_stmt branch July 18, 2017 19:03


		### Update All Rows

		{{site.data.alerts.callout_danger}}If you don't use the WHERE clause to specify the rows to be updated, the values for all rows will be updated.{{site.data.alerts.end}}


		### Update and Return Values

		In this example, the `RETURNING` clause returns the `id` values of the rows updated, which are generated server-side by the `unique_rowid()` function. The language-specific versions assume that you have installed the relevant [client drivers](install-client-drivers.html).


		In this example, the `RETURNING` clause returns the `id` values of the rows updated, which are generated server-side by the `unique_rowid()` function. The language-specific versions assume that you have installed the relevant [client drivers](install-client-drivers.html).

		{{site.data.alerts.callout_success}}This use of <code>RETURNING</code> mirrors the behavior of MySQL's <code>last_insert_id()</code> function.{{site.data.alerts.end}}


		</section>

		<section class="filter-content" markdown="1" data-scope="ruby">

Completed Update statement documentation #1707

Completed Update statement documentation #1707

Uh oh!

Conversation

Amruta-Ranade commented Jul 13, 2017

Uh oh!

cockroach-teamcity commented Jul 13, 2017

Uh oh!

cockroach-teamcity commented Jul 13, 2017

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!

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!

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!

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!

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 Jul 14, 2017

Amruta-Ranade commented Jul 14, 2017 •

edited

Loading