Skip to content

Enhance various statements and diagrams#2762

Merged
jseldess merged 1 commit intocockroachdb:masterfrom
knz:20180321-improve-diagrams
Apr 2, 2018
Merged

Enhance various statements and diagrams#2762
jseldess merged 1 commit intocockroachdb:masterfrom
knz:20180321-improve-diagrams

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Mar 21, 2018

Fixes #2719.
Fixes #2519.
Fixes #2182.
Fixes #2221.
Fixes #2350.
Fixes #2797.
First step towards fixing #534.

tl;dr: fix confusion, better document joins, fix upsert/delete/insert/update.

Prior to this patch, the documentation was confusing "SELECT
statements" and "selection clauses". This confusion was most visible in the documentation for INSERT and UPSERT, which incorrectly specified that these statements take a selection clause as operand, whereas they support much more than that.

The documentation was also incorrectly listing set operations (EXCEPT/UNION etc) as operators of SELECT in particular, whereas they actually work also with any selection clause.

The root cause of this sorry state of affairs was caused by a single
misconception: there is really no such thing as a "SELECT statement"
in the SQL grammar; instead there are various relational expressions
and operators, and selection clauses are just a particular instance of relational expressions.

This patch enhances this situation as follows:

  • The page about a "SELECT statement" is deleted. It is replaced by a
    page titled "Simple SELECT clause", to properly highlight it is
    merely an instance of a selection clause (albeit with a reminder
    that it happens to also be valid as standalone statement).

  • To ensure the basic SELECT syntax remains the "main SELECT doc page"
    for newcomers, the top-level links in navigation for the SELECT
    keyword are changed to point to the page on the simple SELECT
    clause.

  • A new page "Relational Expressions" is introduced, which explains
    properly the concept of "Statement-like Query". The pages about
    other statements that require a statement-like query are modified to
    point to this new page.

  • The difference between "table expressions", "selection clauses" and
    "statement-like queries" is repeatedly clarified in numerous places,
    to ensure that clarity is pervasively maintained throughout the
    docs.

Additionally, the syntax diagrams for all these constructs are updated to more accurately reflect reality. Some more documentation for the various JOIN operators is added.

To review this patch, the reviewer is invited to first proceed in a
holistic manner and navigate the result of the changes with the eyes
of a newcomer:

  1. Land on the "Simple SELECT clause" page; ensure that it makes
    sense, including its simplified syntax diagram.

  2. Check out the page on "Table Expressions", which documents what can
    fit in the FROM clause of a simple SELECT clause. Check that it
    makes sense, including the new syntax diagrams.

  3. Check the docs for INSERT/UPDATE/UPSERT/DELETE and check they make
    sense, including the new syntax diagrams.

  4. Navigate to the page on "Relational Expressions" and check that it
    makes sense, including the new syntax diagrams.

  5. Review the new page "Limiting Query Results" linked from there.

Once the reviewer satisfies themselves that the relationship between
the pages and the syntax diagrams make sense, the line-by-line diff
can be reviewed for further edits.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch 3 times, most recently from 0938462 to f05055a Compare March 21, 2018 22:23
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from f05055a to a31ffdf Compare March 21, 2018 22:34
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from a31ffdf to 1259d3f Compare March 21, 2018 22:47
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch 2 times, most recently from 5a2a6ab to bc2ca99 Compare March 21, 2018 23:23
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch 2 times, most recently from 37d7fb9 to f23efd1 Compare March 25, 2018 14:37
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from f23efd1 to be80969 Compare March 25, 2018 20:25
@knz knz changed the title [WIP] Improve various diagrams Enhance various statements and diagrams Mar 25, 2018
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from be80969 to 27213f9 Compare March 25, 2018 20:51
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from 27213f9 to 96adf10 Compare March 25, 2018 21:00
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from 96adf10 to 790a73d Compare March 25, 2018 21:08
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz requested a review from jseldess March 25, 2018 21:10
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Mar 25, 2018

@jseldess please check the PR description for suggestions on how to review this. I suggest we spend our scheduled time together later this week to do a live review.

@jseldess
Copy link
Copy Markdown
Contributor

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


v2.0/common-table-expressions.md, line 22 at r2 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

As I explained to Rich earlier,

is not really appropriate: it has semantic value

The docs wiki page on these diagrams was recommending the use of <section>, I just updated it to recommend <div>.

Thanks, @knz and @rmloveland. There are some strange edge cases where we've only been able to get the markdown to convert properly by using <section> instead of <div>. This has come up here, for example: #2146 (comment). In any case, it does sound best to recommend <div>. When we run into trouble, we can look more closely.


v2.0/common-table-expressions.md, line 56 at r2 (raw file):

Previously, knz (kena) wrote…

I have learned this style from some books, but you can see some example online:
http://sqlite.org/sqllogictest/artifact/8bf320a7c5bd7456

It makes the queries much easier to read, by ensuring that the important part of the queries (the operands to various clauses, and thus not the keywords) are all aligned on the same column.

Thanks for the context. We should definitely include conventions around multi-line sql samples in the style guide that @lhirata will be driving in the 2.1 release cycle. Lauren, please use Raphael as a resource for that.


v2.0/relational-expressions.md, line 13 at r1 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

Instead of this overview section, I think we should add the other flavors of relational expressions as sections on this page, providing the high-level info you have here with links away to the more details docs.

@knz, you didn't respond this comment, so if there's time, we can discuss in our meeting today. This is a nit, really, and not something that needs to be done now.


v2.0/select-clause.md, line 86 at r1 (raw file):

Previously, knz (kena) wrote…

Done, but I'd like to highlight I merely copied this over from select.md.

Thanks, @knz, and apologies for the current inconsistencies across our docs. As mentioned elsewhere, we're going to work out more thorough style guidelines/conventions in the 2.1 cycle.


v2.0/selection-clauses.md, line 114 at r1 (raw file):

Previously, knz (kena) wrote…

There are several occurrences out of this PR. I changed them as well.

I am not totally fond of this change however. I have never seen this spelled this way.

Yeah, this is pure preference. Both forms are correct, though the hyphenated form is more widely used, I think.


v2.0/subqueries.md, line 29 at r1 (raw file):

Previously, knz (kena) wrote…

See my other comment.

In a lot of cases, you have just left-aligned all lines but the first, so I'm not really clear on when you do that vs. indent. But this is not worth a lot of time right now.


Comments from Reviewable

Comment thread v2.0/select-clause.md
title: Simple SELECT Clause
summary: The Simple SELECT clause loads or computes data from various sources.
toc: false
redirect-from: select.html
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One more field to add to the front-matter to make our version switcher work between 1.1 and 2.0:

key: select.html

summary: Scalar expressions allow the computation of new values from basic parts.
toc: false
redirect-from: sql-expressions.html
---
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One more field to add to get our version selector to work between 1.1 and 2.0:

key: sql-expressions.html

@jseldess
Copy link
Copy Markdown
Contributor

:lgtm:!!!!!! with a few minor edits left.


Comments from Reviewable

@knz knz force-pushed the 20180321-improve-diagrams branch 3 times, most recently from c1e5a73 to 4efd6ca Compare March 31, 2018 16:22
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Mar 31, 2018

Improved, RFA(hopefully last)L


Review status: 0 of 70 files reviewed at latest revision, 10 unresolved discussions.


v2.0/common-table-expressions.md, line 22 at r2 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

Thanks, @knz and @rmloveland. There are some strange edge cases where we've only been able to get the markdown to convert properly by using <section> instead of <div>. This has come up here, for example: #2146 (comment). In any case, it does sound best to recommend <div>. When we run into trouble, we can look more closely.

Will follow up in #2819.


v2.0/common-table-expressions.md, line 56 at r2 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

Thanks for the context. We should definitely include conventions around multi-line sql samples in the style guide that @lhirata will be driving in the 2.1 release cycle. Lauren, please use Raphael as a resource for that.

Let's possibly follow up in #2819 too.


v2.0/select-clause.md, line 5 at r3 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

One more field to add to the front-matter to make our version switcher work between 1.1 and 2.0:

key: select.html

Done.


v2.0/relational-expressions.md, line 13 at r1 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

@knz, you didn't respond this comment, so if there's time, we can discuss in our meeting today. This is a nit, really, and not something that needs to be done now.

I have reworked this page and changed the intro to condense it to its bare essentials. I think the result is more clear this way.


v2.0/scalar-expressions.md, line 6 at r3 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

One more field to add to get our version selector to work between 1.1 and 2.0:

key: sql-expressions.html

Done.


Comments from Reviewable

@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch from 4efd6ca to 5e99890 Compare March 31, 2018 19:35
@cockroach-teamcity
Copy link
Copy Markdown
Member

@jseldess
Copy link
Copy Markdown
Contributor

jseldess commented Apr 1, 2018

Thank you for iterating, @knz. I'm now concerned about the proliferation of terms we use to refer to the same thing: stand-alone queries, statement-like queries, statement-like relational expressions, and possibly others. These terms are not intuitive in themselves, and don't seem to appear in the mysql or postgres docs, so if we're going to use a term that is not already conventional, we need to choose 1 and stick with it. Can you please make that change? I'd probably suggest "statement-like queries" for now.

More broadly, on further review and consideration, I have to admit that I'm still confused and a bit concerned about this level of abstraction. I think, if we wanted to keep things simpler, we could get away with covering these semantics just on the selection clauses page, given the definition of statement-like queries as "a group one or more selection clause with set operations". But it is late in the game to request such a change, and we do need to merge soon, so I'm ok holding off and looping other people in for perspective post-2.0 release.


Comments from Reviewable

@knz knz force-pushed the 20180321-improve-diagrams branch from 5e99890 to 2d6815c Compare April 1, 2018 17:54
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Apr 1, 2018

Jesse, thanks for the feedback. I agree with it and I am glad that we could get other more superficial issues out of the way earlier in the review process to reveal the more important issues.

I have decided to collapse the pages on set operations, selection clauses and relational expressions into a single page "selection queries". I think this term is both suitably general, simple and accurate.

Let me know what you think.


Review status: 0 of 71 files reviewed at latest revision, 10 unresolved discussions.


Comments from Reviewable

@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz knz force-pushed the 20180321-improve-diagrams branch 2 times, most recently from 251189a to 16b5ff4 Compare April 1, 2018 18:23
@cockroach-teamcity
Copy link
Copy Markdown
Member

@cockroach-teamcity
Copy link
Copy Markdown
Member

@jseldess
Copy link
Copy Markdown
Contributor

jseldess commented Apr 1, 2018

This LGTM! Just a few final nits. Thank you for putting in so much time and effort.


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


v2.0/selection-queries.md, line 11 at r4 (raw file):

Selection queries read and process data in CockroachDB.  They are more
general than [simple `SELECT` clauses](select-clause.html): they can
group one or more [selection clause](#selection-clauses) with [set

one or more selection clauses


v2.0/selection-queries.md, line 15 at r4 (raw file):

ordering](query-order.html) or [row limit](limit-offset.html).

Selection Queries can occur:

Let's use Selection queries, as you've done in body text elsewhere.


Comments from Reviewable

**tl;dr**: fix confusion, better document joins, fix upsert/delete/insert/update.

Prior to this patch, the documentation was confusing "SELECT
statements" and "selection clauses". This confusion was most visible in the documentation for INSERT and UPSERT, which incorrectly specified that these statements take a selection clause as operand, whereas they support much more than that.

The documentation was also incorrectly listing set operations (EXCEPT/UNION etc) as operators of SELECT in particular, whereas they actually work also with any selection clause.

The root cause of this sorry state of affairs was caused by a single
misconception: there is really no such thing as a "SELECT statement"
in the SQL grammar; instead there are various relational expressions
and operators, and selection clauses are just a particular instance of relational expressions.

This patch enhances this situation as follows:

- The page about a "SELECT statement" is deleted. It is replaced by a
  page titled "Simple SELECT clause", to properly highlight it is
  merely an instance of a selection clause (albeit with a reminder
  that it happens to also be valid as standalone statement).

- To ensure the basic SELECT syntax remains the "main SELECT doc page"
  for newcomers, the top-level links in navigation for the `SELECT`
  keyword are changed to point to the page on the simple SELECT
  clause.

- A new page "Statement-Like Relational Expressions" is introduced,
  which explains properly what can become a subquery. The pages about
  other statements that require a statement-like query are modified to
  point to this new page.

- The difference between "table expressions", "selection clauses" and
  "statement-like queries" is repeatedly clarified in numerous places,
  to ensure that clarity is pervasively maintained throughout the
  docs.

Additionally, the syntax diagrams for all these constructs are updated to more accurately reflect reality. Some more documentation for the various `JOIN` operators is added, as well as for subqueries. The term "value expression" is removed in favor of the more correct "scalar expression".

To review this patch, the reviewer is invited to first proceed in a
holistic manner and navigate the result of the changes with the eyes
of a newcomer:

1. Land on the "Simple SELECT clause" page; ensure that it makes
   sense, including its simplified syntax diagram.

2. Check out the page on "Table Expressions", which documents what can
   fit in the FROM clause of a simple SELECT clause. Check that it
   makes sense, including the new syntax diagrams.

3. Check the new page on "Join Expressions". Check that it makes sense, including the new syntax diagrams and "performance best practices".

4. Check the docs for INSERT/UPDATE/UPSERT/DELETE and check they make
   sense, including the new syntax diagrams.

5. Navigate to the page on "Relational Expressions" and check that it
   makes sense, including the new syntax diagrams.

6. Review the new page "Limiting Query Results" linked from there.

7. Check The new page "Subqueries".

Once the reviewer satisfies themselves that the relationship between
the pages and the syntax diagrams make sense, the line-by-line diff
can be reviewed for further edits.
@knz knz force-pushed the 20180321-improve-diagrams branch from 16b5ff4 to dc60e25 Compare April 1, 2018 21:57
@cockroach-teamcity
Copy link
Copy Markdown
Member

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Apr 1, 2018

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


v2.0/selection-queries.md, line 11 at r4 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

one or more selection clauses

Done.


v2.0/selection-queries.md, line 15 at r4 (raw file):

Previously, jseldess (Jesse Seldess) wrote…

Let's use Selection queries, as you've done in body text elsewhere.

Done.


Comments from Reviewable

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Apr 1, 2018

Ok fixed. I surmise we can merge this now?

@jseldess
Copy link
Copy Markdown
Contributor

jseldess commented Apr 2, 2018

Yes. Thank you, @knz!

@jseldess jseldess merged commit 93366de into cockroachdb:master Apr 2, 2018
@knz knz deleted the 20180321-improve-diagrams branch September 21, 2018 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants