[SPARK-31110][DOCS][SQL] refine sql doc for SELECT by cloud-fan · Pull Request #27866 · apache/spark

cloud-fan · 2020-03-10T15:16:05Z

What changes were proposed in this pull request?

A few improvements to the sql ref SELECT doc:

correct the syntax of SELECT query
correct the default of null sort order
correct the GROUP BY syntax
several minor fixes

Why are the changes needed?

refine document

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

N/A

cloud-fan · 2020-03-10T15:16:57Z

docs/sql-ref-syntax-qry-select-clusterby.md

  (the "License"); you may not use this file except in compliance with
  the License.  You may obtain a copy of the License at
- 
+


My editor removes trailing spaces automatically

cloud-fan · 2020-03-10T15:17:19Z

docs/sql-ref-syntax-qry-select-groupby.md

 {% highlight sql %}
 GROUP BY [ GROUPING SETS grouping_sets ] group_expression [ , group_expression [ , ... ] ]
-    [ ( WITH ROLLUP | WITH CUBE | GROUPING SETS grouping_sets ) ) ]
+    [ ( WITH ROLLUP | WITH CUBE | GROUPING SETS grouping_sets ) ]


remove the extra )

cloud-fan · 2020-03-10T15:18:30Z

docs/sql-ref-syntax-qry-select-groupby.md

-    <code> ((warehouse, product), (warehouse), ())</code>.
+    Specifies multiple levels of aggregations in a single statement. This clause is used to compute aggregations
+    based on multiple grouping sets. <code>ROLLUP</code> is a shorthand for <code>GROUPING SETS</code>. For example,
+    <code>GROUP BY warehouse, product WITH ROLLUP</code> is equivalent to <code>GROUP BY GROUPING SETS


GROUP BY GROUPING SETS ... is the same as GROUP BY a, b GROUPING SETS but shorter.

cloud-fan · 2020-03-10T15:19:37Z

docs/sql-ref-syntax-qry-select-orderby.md

-    sort order is <code>DESC</code>.<br><br>
+    Optionally specifies whether NULL values are returned before/after non-NULL values. If
+    <code>null_sort_order</code> is not specified, then NULLs sort first if sort order is
+    <code>ASC</code> and NULLS sort last if sort order is <code>DESC</code>.<br><br>


We don't need to mention In Spark, NULL values are considered to be lower than any non-NULL values, as it's not related to the user-facing behavior.

cloud-fan · 2020-03-10T15:19:56Z

docs/sql-ref-syntax-qry-select-orderby.md

+    <code>ASC</code> and NULLS sort last if sort order is <code>DESC</code>.<br><br>
    <ol>
-      <li> If <code>NULLS FIRST</code> (the default) is specified, then NULL values are returned first 
+      <li> If <code>NULLS FIRST</code> is specified, then NULL values are returned first


the default value depends on the sort order.

cloud-fan · 2020-03-10T15:20:58Z

docs/sql-ref-syntax-qry-select.md

 {% highlight sql %}
 [ WITH with_query [ , ... ] ]
+select_statement [ { UNION | INTERSECT | EXCEPT } select_statement, ... ]
+[ ORDER BY { expression [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [ , ...] } ]


according to our g4 file, a query can have multiple SELECT but only one ORDER BY/SORT BY/...

cloud-fan · 2020-03-10T15:21:49Z

@dilipbiswal @dongjoon-hyun @viirya @maropu

SparkQA · 2020-03-10T15:29:53Z

Test build #119622 has finished for PR 27866 at commit 7ba555e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-03-10T17:40:38Z

docs/sql-ref-syntax-qry-select.md

 ### Syntax
 {% highlight sql %}
 [ WITH with_query [ , ... ] ]
+select_statement [ { UNION | INTERSECT | EXCEPT } select_statement, ... ]


[ ALL | DISTINCT ] seems to be missed while transition.

It's defined in select_statement: https://github.com/apache/spark/pull/27866/files#diff-bc52347d9c1373190bce4389d3066ad5R40

Hi, @cloud-fan . They are not the same. I mean UNION ALL, not SELECT ALL.

scala> sql("select 1 union all (select all 1)").show +---+ | 1| +---+ | 1| | 1| +---+

dongjoon-hyun · 2020-03-10T17:41:16Z

docs/sql-ref-syntax-qry-select.md

 [ WITH with_query [ , ... ] ]
+select_statement [ { UNION | INTERSECT | EXCEPT } select_statement, ... ]
+[ ORDER BY { expression [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [ , ...] } ]
+[ SORT  BY { expression [ ASC | DESC ] [ NULLS { FIRST | LAST } ] [ , ...] } ]


- SORT BY + SORT BY

dongjoon-hyun

+1, LGTM (with two minor comments.)
Thank you, @cloud-fan .

maropu · 2020-03-10T22:52:19Z

docs/sql-ref-syntax-qry-select-limit.md

  |Shone S| 16|
  +-------+---+
+
+-- A non-foldable expression as an input to limit is not allowed.


nit: limit -> LIMIT?

maropu

The change can make the docs cleaner! LGTM except the existing comments.

viirya · 2020-03-10T23:29:37Z

docs/sql-ref-syntax-qry-select-groupby.md

    <b>Syntax:</b>
      <code>
-        ( () | ( expression [ , ... ] ) )
+        ( '()' | ( expression [ , ... ] ) )


The syntax here indicates one group set or more? In above the syntax is GROUP BY [ GROUPING SETS grouping_sets ] group_expression [ , group_expression [ , ... ] ].

For example, group_expression here is for just one group expression.

SparkQA · 2020-03-11T04:43:43Z

Test build #119646 has finished for PR 27866 at commit d40afe2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-03-11T08:06:48Z

docs/sql-ref-syntax-qry-select-groupby.md


 ### Syntax
 {% highlight sql %}
-GROUP BY [ GROUPING SETS grouping_sets ] group_expression [ , group_expression [ , ... ] ]


This was wrong.

@viirya can you help review this file? thanks!

Sorry for late. This change LGTM.

SparkQA · 2020-03-11T08:23:03Z

Test build #119663 has finished for PR 27866 at commit 987034e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-03-11T16:51:16Z

@cloud-fan . Please don't forget the following

[SPARK-31110][DOCS][SQL] refine sql doc for SELECT #27866 (comment)

SparkQA · 2020-03-11T17:31:17Z

Test build #119678 has finished for PR 27866 at commit 5742080.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun

+1, LGTM. Merged to master/3.0. Thank you all!

### What changes were proposed in this pull request? A few improvements to the sql ref SELECT doc: 1. correct the syntax of SELECT query 2. correct the default of null sort order 3. correct the GROUP BY syntax 4. several minor fixes ### Why are the changes needed? refine document ### Does this PR introduce any user-facing change? N/A ### How was this patch tested? N/A Closes #27866 from cloud-fan/doc. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 0f0ccda) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? A few improvements to the sql ref SELECT doc: 1. correct the syntax of SELECT query 2. correct the default of null sort order 3. correct the GROUP BY syntax 4. several minor fixes ### Why are the changes needed? refine document ### Does this PR introduce any user-facing change? N/A ### How was this patch tested? N/A Closes apache#27866 from cloud-fan/doc. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

refine sql doc for SELECT

7ba555e

cloud-fan commented Mar 10, 2020

View reviewed changes

dongjoon-hyun reviewed Mar 10, 2020

View reviewed changes

dongjoon-hyun approved these changes Mar 10, 2020

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-31110][SQL] refine sql doc for SELECT~~ [SPARK-31110][DOCS][SQL] refine sql doc for SELECT Mar 10, 2020

dongjoon-hyun added DOCUMENTATION SQL labels Mar 10, 2020

maropu reviewed Mar 10, 2020

View reviewed changes

maropu approved these changes Mar 10, 2020

View reviewed changes

address comments

d40afe2

viirya reviewed Mar 11, 2020

View reviewed changes

clarify the GROUPING SETS syntax

987034e

cloud-fan commented Mar 11, 2020

View reviewed changes

fix mistake

5742080

dongjoon-hyun approved these changes Mar 11, 2020

View reviewed changes

dongjoon-hyun closed this in 0f0ccda Mar 11, 2020

		(the "License"); you may not use this file except in compliance with
		the License. You may obtain a copy of the License at

Conversation

cloud-fan commented Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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!

cloud-fan commented Mar 10, 2020

Uh oh!

SparkQA commented Mar 10, 2020

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!

dongjoon-hyun Mar 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viirya Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Mar 11, 2020

Uh oh!

dongjoon-hyun commented Mar 11, 2020

Uh oh!

SparkQA commented Mar 11, 2020

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

cloud-fan commented Mar 10, 2020 •

edited

Loading

dongjoon-hyun Mar 10, 2020 •

edited

Loading

viirya Mar 13, 2020 •

edited

Loading