Skip to content

Conversation

@GuoPhilipse
Copy link
Member

@GuoPhilipse GuoPhilipse commented Jul 9, 2020

What changes were proposed in this pull request?

update sql-ref docs, the following key words will be added in this PR.

CASE/ELSE
WHEN/THEN
MAP KEYS TERMINATED BY
NULL DEFINED AS
LINES TERMINATED BY
ESCAPED BY
COLLECTION ITEMS TERMINATED BY
PIVOT
LATERAL VIEW OUTER?
ROW FORMAT SERDE
ROW FORMAT DELIMITED
FIELDS TERMINATED BY
IGNORE NULLS
FIRST
LAST

Why are the changes needed?

let more users know the sql key words usage

Does this PR introduce any user-facing change?

image
image
image
image

How was this patch tested?

No

@GuoPhilipse GuoPhilipse changed the title [SPARK-31753][SQL][DOCS]Add missing keywords [SPARK-31753][SQL][DOCS][WIP]Add missing keywords Jul 9, 2020
@maropu
Copy link
Member

maropu commented Jul 10, 2020

cc: @huaxingao

@maropu
Copy link
Member

maropu commented Jul 10, 2020

Thanks for the work, @GuoPhilipse ! Please list up the keywords in the PR description that this PR will add.

@GuoPhilipse
Copy link
Member Author

Thanks for the work, @GuoPhilipse ! Please list up the keywords in the PR description that this PR will add.
sure, will add it later.

@maropu
Copy link
Member

maropu commented Jul 17, 2020

btw, could you update the PR description, too? e.g., remove FROM

@GuoPhilipse
Copy link
Member Author

btw, could you update the PR description, too? e.g., remove FROM

sure ,wil update soon.

@huaxingao
Copy link
Contributor

also edit the menu-sql.yaml to update the sidebar menu?

@huaxingao
Copy link
Contributor

@GuoPhilipse Sorry I have more nitpicks on grammar. I know this is very tedious. I went through all these when I did the doc PRs. Thanks a lot for doing this!
I have no more comments :)

@GuoPhilipse
Copy link
Member Author

Thanks @maropu @huaxingao, really appreciated by your earnest and carefull review. I have leant a lot from this PR. Thanks for your time.
BTW @maropu do you have any new comments?


```sql
PIVOT ( { aggregate_expression [ AS aggregate_expression_alias ] } [ , ... ]
FOR column_list IN ( expression_list ) )
Copy link
Member

Choose a reason for hiding this comment

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

Specifies an aggregate function name (MIN, MAX, COUNT, SUM, AVG, etc.).
Note that `FIRST` and `LAST` have an optional `IGNORE NULLS` clause; when the option specified,
it will returns the first or last value that is not null (or null if all values are null).
**Syntax:** `[ FIRST | LAST ] ( [ distinct ] expression [ IGNORE NULLS ] ) [ FILTER ( WHERE boolean_expression ) ]`
Copy link
Member

@maropu maropu Jul 21, 2020

Choose a reason for hiding this comment

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

We still need **Syntax:** [ FIRST | LAST ] ( [ distinct ] expression [ IGNORE NULLS ] ) [ FILTER ( WHERE boolean_expression ) ]? It looks verbose to me though.

Copy link
Member Author

Choose a reason for hiding this comment

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

now have have aggregate functions above and examples in the end, looks enough,let me move it

@maropu
Copy link
Member

maropu commented Jul 21, 2020

Thanks for the work, @GuoPhilipse ! Looks ok except for one comment. Could you check for the final sign-off? @srowen @dongjoon-hyun @huaxingao @dilipbiswal Note: since all the SQL syntaxes in this update has been implemented in 3.0.0, we can merge this into branch-3.0.

@maropu
Copy link
Member

maropu commented Jul 21, 2020

@GuoPhilipse Could you generate HTML docs and check if all things are okay for this udpate?
In most PRs for doc updates, the screenshots of HTML docs are often attached in the PR description, e.g., #28672

@maropu
Copy link
Member

maropu commented Jul 21, 2020

@GuoPhilipse Btw, we still have more document improvement issues;

  1. Improve the structure of auto-generated built-in function pages in SQL references, https://issues.apache.org/jira/browse/SPARK-31513
  2. Add group tags (ExpressionDescription) to all the built-in functions: (Related JIRA: https://issues.apache.org/jira/browse/SPARK-31429)

As you know, we currently have the two duplicated document pages for built-in functions;

If we finish adding the tags, we might be able to remove the former page. If you're interested in more contributions, feel free to take them over. Its very helpful. cc: @HyukjinKwon @huaxingao

@GuoPhilipse
Copy link
Member Author

@GuoPhilipse Btw, we still have more document improvement issues;

  1. Improve the structure of auto-generated built-in function pages in SQL references, https://issues.apache.org/jira/browse/SPARK-31513
  2. Add group tags (ExpressionDescription) to all the built-in functions: (Related JIRA: https://issues.apache.org/jira/browse/SPARK-31429)

As you know, we currently have the two duplicated document pages for built-in functions;

If we finish adding the tags, we might be able to remove the former page. If you're interested in more contributions, feel free to take them over. Its very helpful. cc: @HyukjinKwon @huaxingao

Thanks @maropu , will glad to take more look on this.

@GuoPhilipse
Copy link
Member Author

@GuoPhilipse Could you generate HTML docs and check if all things are okay for this udpate?
In most PRs for doc updates, the screenshots of HTML docs are often attached in the PR description, e.g., #28672
will append later

@maropu
Copy link
Member

maropu commented Jul 23, 2020

ok to test

@maropu
Copy link
Member

maropu commented Jul 23, 2020

Note: If nobody has comments, this is just a doc improvement, so I will merge in a few days.

@SparkQA
Copy link

SparkQA commented Jul 23, 2020

Test build #126368 has finished for PR 29056 at commit ffa0603.

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

@maropu maropu closed this in 8de4333 Jul 28, 2020
maropu pushed a commit that referenced this pull request Jul 28, 2020
### What changes were proposed in this pull request?
update sql-ref docs, the following key words will be added in this PR.

CASE/ELSE
WHEN/THEN
MAP KEYS TERMINATED BY
NULL DEFINED AS
LINES TERMINATED BY
ESCAPED BY
COLLECTION ITEMS TERMINATED BY
PIVOT
LATERAL VIEW OUTER?
ROW FORMAT SERDE
ROW FORMAT DELIMITED
FIELDS TERMINATED BY
IGNORE NULLS
FIRST
LAST

### Why are the changes needed?
let more users know the sql key words usage

### Does this PR introduce _any_ user-facing change?
![image](https://user-images.githubusercontent.com/46367746/88148830-c6dc1f80-cc31-11ea-81ea-13bc9dc34550.png)
![image](https://user-images.githubusercontent.com/46367746/88148968-fb4fdb80-cc31-11ea-8649-e8297cf5813e.png)
![image](https://user-images.githubusercontent.com/46367746/88149000-073b9d80-cc32-11ea-9aa4-f914ecd72663.png)
![image](https://user-images.githubusercontent.com/46367746/88149021-0f93d880-cc32-11ea-86ed-7db8672b5aac.png)

### How was this patch tested?
No

Closes #29056 from GuoPhilipse/add-missing-keywords.

Lead-authored-by: GuoPhilipse <[email protected]>
Co-authored-by: GuoPhilipse <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
(cherry picked from commit 8de4333)
Signed-off-by: Takeshi Yamamuro <[email protected]>
@maropu
Copy link
Member

maropu commented Jul 28, 2020

Merged to master/branch-3.0. Thanks, @GuoPhilipse !

| ( col_name1, col_name2, ... ) ]
[ ROW FORMAT row_format ]
[ STORED AS file_format ]
[ LOCATION path ]
Copy link
Member

Choose a reason for hiding this comment

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

The bucketSpec is still missing in CREATE HIVE FORMAT table, right?

    [ CLUSTERED BY ( col_name3, col_name4, ... ) 
        [ SORTED BY ( col_name [ ASC | DESC ], ... ) ] 
        INTO num_buckets BUCKETS ]

Copy link
Member

Choose a reason for hiding this comment

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

Any reason we did not add it? @huaxingao @GuoPhilipse

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, we missed that.
@GuoPhilipse Could you please have a follow-up to add bucketSpec? Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we need it, will add it soon.

@GuoPhilipse GuoPhilipse deleted the add-missing-keywords branch September 27, 2020 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants