Skip to content

Conversation

lnhsingh
Copy link
Contributor

@lnhsingh lnhsingh commented Mar 8, 2018

Closes #1963.

@lnhsingh lnhsingh requested a review from jseldess March 8, 2018 22:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@cockroach-teamcity
Copy link
Member

v1.0/explain.md Outdated
- [`ALTER TABLE`](alter-table.html)
- [`CREATE`](sql-grammar.html#create_stmt)
- [`DELETE`](delete.html)
- [`HELP`](sql-grammar.html#help_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove HELP, it was not meant to be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v1.0/explain.md Outdated
- [`INSERT`](insert.html)
- [`SELECT`](select.html)
- [`SHOW`](sql-grammar.html#show_stmt)
- [`SPLIT`](sql-grammar.html#split_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no "SPLIT" statement, it's a special form of ALTER.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v1.0/explain.md Outdated

You can use `EXPLAIN` on the following statements:

- [`ALTER TABLE`](alter-table.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all the forms of ALTER can be explained, not just ALTER TABLE. Or perhaps that was not true in 1.0?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also put the various ALTER statements on the same line with proper doc links like this:

- [`ALTER TABLE`](alter-table.html), [`ALTER INDEX`](alter-index.html), ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the full SQL grammar, only ALTER TABLE is explainable in 1.0. I'll update the following versions with how you suggested though.

v1.0/explain.md Outdated
- [`SELECT`](select.html)
- [`SHOW`](sql-grammar.html#show_stmt)
- [`SPLIT`](sql-grammar.html#split_stmt)
- [`UPDATE`](update.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add EXPLAIN to this list too (EXPLAIN is explainable)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v2.0/explain.md Outdated
- [`SET CLUSTER SETTING`](set-cluster-setting.html)
- [`SHOW`](sql-grammar.html#show_stmt)
- [`UPDATE`](update.html)
- [`UPSERT`](upsert.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add EXPLAIN to this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

v1.1/explain.md Outdated
- [`EXECUTE`](sql-grammar.html#execute_stmt)
- [`IMPORT`](sql-grammar.html#import_stmt)
- [`PAUSE JOB`](sql-grammar.html#pause_stmt)
- [`RESET`](sql-grammar.html#reset_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

link reset-vars.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v1.1/explain.md Outdated
- [`PAUSE JOB`](sql-grammar.html#pause_stmt)
- [`RESET`](sql-grammar.html#reset_stmt)
- [`RESTORE`](restore.html)
- [`RESUME JOB`](sql-grammar.html#resume_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

link resume-job.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v1.1/explain.md Outdated
- [`RESTORE`](restore.html)
- [`RESUME JOB`](sql-grammar.html#resume_stmt)
- [`SELECT`](select.html)
- [`SET SESSION`](sql-grammar.html#set_session_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

link set-vars.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v1.1/explain.md Outdated
- [`SELECT`](select.html)
- [`SET SESSION`](sql-grammar.html#set_session_stmt)
- [`SET CLUSTER SETTING`](set-cluster-settings.html)
- [`SHOW`](sql-grammar.html#show_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

list the various SHOW pages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v2.0/explain.md Outdated

<div id="toc"></div>

## Explainable Statements

- [`ALTER`](sql-grammar.html#alter_stmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto precise page links as for 1.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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.

Only a few suggestions on top of @knz's.


<div id="toc"></div>

## Explainable Statements

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's us an intro sentence like you have in the 1.1 version of the page:

You can use EXPLAIN on the following statements:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v1.0/explain.md Outdated
@@ -4,10 +4,24 @@ summary: The EXPLAIN statement provides information you can use to optimize SQL
toc: false
---

The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan to execute [`DELETE`](delete.html), [`INSERT`](insert.html), [`SELECT`](select.html) or [`UPDATE`](update.html) statements. You can then use this information to optimize those queries.
The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan to execute the [explainable statements](#explainable-statments). You can then use this information to optimize those queries.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: to execute the explainable statements > for an explainable statement.

Also, I'd tweak the second sentence a bit: You can then use this information to optimize the query.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please apply this change to other versions of the page as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

v1.0/explain.md Outdated
@@ -4,10 +4,23 @@ summary: The EXPLAIN statement provides information you can use to optimize SQL
toc: false
---

The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan to execute [`DELETE`](delete.html), [`INSERT`](insert.html), [`SELECT`](select.html) or [`UPDATE`](update.html) statements. You can then use this information to optimize those queries.
The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan for an [explainable statements](#explainable-statements). You can then use this information to optimize the query.
Copy link
Contributor

Choose a reason for hiding this comment

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

explainable statements > explainable statement

v1.1/explain.md Outdated
@@ -4,10 +4,34 @@ summary: The EXPLAIN statement provides information you can use to optimize SQL
toc: false
---

The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan to execute [`DELETE`](delete.html), [`INSERT`](insert.html), [`SELECT`](select.html) or [`UPDATE`](update.html) statements. You can then use this information to optimize those queries.
The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan for an [explainable statements](#explainable-statements). You can then use this information to optimize the query.
Copy link
Contributor

Choose a reason for hiding this comment

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

explainable statements > explainable statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

v2.0/explain.md Outdated
@@ -4,10 +4,35 @@ summary: The EXPLAIN statement provides information you can use to optimize SQL
toc: false
---

The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan to execute [`DELETE`](delete.html), [`INSERT`](insert.html), [`SELECT`](select.html) or [`UPDATE`](update.html) statements. You can then use this information to optimize those queries.
The `EXPLAIN` [statement](sql-statements.html) returns CockroachDB's query plan for an [explainable statements](#explainable-statements). You can then use this information to optimize the query.
Copy link
Contributor

Choose a reason for hiding this comment

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

explainable statements > explainable statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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.

Something in the v2.0 page seems to have messed up the page after the diagram: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/f631d5df0657fbfdedf70a32f96abcaf6e3a7ed1/dev/explain.html#synopsis

Can you wrap the diagram in <section> tags and see if that fixes the problem?

<section>{% include sql/{{ page.version.version }}/diagrams/explain.html %}</section>

@cockroach-teamcity
Copy link
Member

1 similar comment
@cockroach-teamcity
Copy link
Member

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.

Seems like we need to update the explainable_stmt description under "Parameters" and the See Also list as well, on all versions of the page. For See Also, no need to list all statements that can be explained, I think.

@cockroach-teamcity
Copy link
Member

@cockroach-teamcity
Copy link
Member

@jseldess
Copy link
Contributor

LGTM, @lhirata!

@jseldess jseldess merged commit 8698fc7 into master Mar 12, 2018
@jseldess jseldess deleted the fixit-explain branch March 12, 2018 17:15
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