Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-31434][SQL][DOCS] Drop builtin function pages from SQL references #28203

Closed
wants to merge 1 commit into from

Conversation

maropu
Copy link
Member

@maropu maropu commented Apr 13, 2020

What changes were proposed in this pull request?

This PR intends to drop the built-in function pages from SQL references. We've already had a complete list of built-in functions in the API documents.

See related discussions for more details:
#28170 (comment)

Why are the changes needed?

For better SQL documents.

Does this PR introduce any user-facing change?

functions

How was this patch tested?

Manually checked.

@maropu
Copy link
Member Author

maropu commented Apr 13, 2020

cc: @huaxingao @HyukjinKwon @srowen

@SparkQA
Copy link

SparkQA commented Apr 13, 2020

Test build #121193 has finished for PR 28203 at commit 9b3b71a.

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

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Apr 13, 2020

+1 as the temporary status.

I will try to get the auto-generation done (SPARK-31429) for Spark 3.0 if nobody takes it .. if we fail, we could do it for Spark 3.1.

@HyukjinKwon
Copy link
Member

From my cursory look, seems good to go.

@huaxingao
Copy link
Contributor

The changes look good. The only problem is that I don't have a place any more to document the window functions. We don't want to list the window function names and descriptions, but we still want to document what window functions are, how to use them and examples, right?

@maropu
Copy link
Member Author

maropu commented Apr 13, 2020

The changes look good. The only problem is that I don't have a place any more to document the window functions. We don't want to list the window function names and descriptions, but we still want to document what window functions are, how to use them and examples, right?

We still need to document them? They are also listed in the auto-generated API doc, right?

@huaxingao
Copy link
Contributor

The auto-generated API doc has the function description, but it doesn't cover the detailed usage such as how to mark the function as window function, how to define the window spec, what's inside the window spec, etc. Window function is quite important and complex, so we need more documentation than function description.
I discussed with @gatorsmile earlier today, if we can't document window function here, he wants me to have detailed documentation for all these window functions in SQL syntax section.

@maropu
Copy link
Member Author

maropu commented Apr 14, 2020

If necessary, how about adding them in sql-ref-syntax-qry-window.md just like table-value functions?

@huaxingao
Copy link
Contributor

Sure. I will document these in SQL syntax section instead.

@maropu
Copy link
Member Author

maropu commented Apr 14, 2020

Thanks!

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Apr 14, 2020

@huaxingao, do you have any page you want to keep here otherwise? If not I will just merge this.

@huaxingao
Copy link
Contributor

No... The main purpose of having a builtin function page is to document window function. Now since there is an alternative place to document this, I am OK to remove this page. Please go ahead to merge. Thanks for checking with me. @HyukjinKwon

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM

@HyukjinKwon
Copy link
Member

Merged to master and branch-3.0.

HyukjinKwon pushed a commit that referenced this pull request Apr 14, 2020
### What changes were proposed in this pull request?

This PR intends to drop the built-in function pages from SQL references. We've already had a complete list of built-in functions in the API documents.

See related discussions for more details:
#28170 (comment)

### Why are the changes needed?

For better SQL documents.

### Does this PR introduce any user-facing change?

![functions](https://user-images.githubusercontent.com/692303/79109009-793e5400-7db2-11ea-8cb7-4c3cf31ccb77.png)

### How was this patch tested?

Manually checked.

Closes #28203 from maropu/DropBuiltinFunctionDocs.

Authored-by: Takeshi Yamamuro <yamamuro@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit 853c6c9)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
@maropu
Copy link
Member Author

maropu commented Apr 14, 2020

Thanks, all.

sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
### What changes were proposed in this pull request?

This PR intends to drop the built-in function pages from SQL references. We've already had a complete list of built-in functions in the API documents.

See related discussions for more details:
apache#28170 (comment)

### Why are the changes needed?

For better SQL documents.

### Does this PR introduce any user-facing change?

![functions](https://user-images.githubusercontent.com/692303/79109009-793e5400-7db2-11ea-8cb7-4c3cf31ccb77.png)

### How was this patch tested?

Manually checked.

Closes apache#28203 from maropu/DropBuiltinFunctionDocs.

Authored-by: Takeshi Yamamuro <yamamuro@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants