[SPARK-31429][SQL][DOC] Automatically generates a SQL document for built-in functions #28224

maropu · 2020-04-15T09:17:11Z

What changes were proposed in this pull request?

This PR intends to add a Python script to generates a SQL document for built-in functions and the document in SQL references.

Why are the changes needed?

To make SQL references complete.

Does this PR introduce any user-facing change?

Yes;

How was this patch tested?

Manually checked and added tests.

maropu · 2020-04-15T09:17:50Z

just like this? @HyukjinKwon @huaxingao

SparkQA · 2020-04-15T09:21:44Z

Test build #121314 has finished for PR 28224 at commit fc9f2d5.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-04-15T09:45:09Z

Test build #121318 has finished for PR 28224 at commit ef6391e.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2020-04-15T09:49:57Z

docs/sql-ref-functions.md

+Spark SQL has some categories of frequently-used built-in functions for aggregtion, arrays/maps, date/timestamp, and JSON data.
+This subsection presents the usages and descriptions of these functions.
+
+ * [Aggregate Functions](sql-ref-functions-builtin.html#aggregate-functions)


Super nit: order of bullets/words in the above statement is different from that in sql-ref-functions-builtin.html. .e. the alphabetical order in sql-ref-functions-builtin.html.

I am neutral on the order (current or consistent among them).

Ah, actually, I sorted them by similar functionality, e.g., array and map. On the other hand, in sql-ref-functions-builtin.html, the script sorts them just by group name.... Yea, we can update the script for sorting them in the same order though, I just want to keep it simple.

huaxingao · 2020-04-15T17:24:34Z

Looks really nice! I love this feature! Thanks a lot! @maropu @HyukjinKwon

huaxingao · 2020-04-15T17:43:02Z

A few comments:

Can we still have window functions here? In SQL Syntax section, I am focusing on SQL syntax, I don't have a list of window functions and their descriptions there.
Can we list other function categories as well? such as Math functions, string functions, sorting functions, udf functions? Also, functions.scala puts Array functions in Collection functions group. Shall we use Collection functions instead of Array functions?
In the left side menu, can we have Built-in Functions and User Defined Functions, and put the three UDFs as subcategories?

maropu · 2020-04-16T00:06:20Z

Can we still have window functions here? In SQL Syntax section, I am focusing on SQL syntax, I don't have a list of window functions and their descriptions there.

Yea, we can do. I'll update later. Thanks!

Can we list other function categories as well? such as Math functions, string functions, sorting functions, udf functions?

Yea, we can do, too. But, in this PR, I just want to focus on the python code and the basic set of built-in functions. As for the other function groups, I think we need to discuss more about which functions should be listed, or not.

Also, functions.scala puts Array functions in Collection functions group. Shall we use Collection functions instead of Array functions?

Yea, I noticed that, but the collection group includes array, map, and JSON functions. I personally think splitting it into the three groups looks easy-to-serach for users. WDYT?

In the left side menu, can we have Built-in Functions and User Defined Functions, and put the three UDFs as subcategories?

The original one had multiple document pages for the functions, so it had one more hierarchy. But, the script generates a single page for them. So, I think we don't need the hierarchy. If we need to improve the structure, feel free to do it as follow-up.

maropu · 2020-04-16T00:36:52Z

Can we still have window functions here? In SQL Syntax section, I am focusing on SQL syntax, I don't have a list of window functions and their descriptions there.

Yea, we can do. I'll update later. Thanks!

Ur, I noticed now that all the window functions have no example tag, e.g., https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/windowExpressions.scala#L428

We need to add examples there before generating the doc.

huaxingao · 2020-04-16T01:36:09Z

We need to add examples there before generating the doc.

I guess no window functions examples there because these examples are a little complicated. Is it OK we just generate the doc without examples?

SparkQA · 2020-04-16T02:11:36Z

Test build #121341 has finished for PR 28224 at commit 3382bf5.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-04-16T02:12:19Z

Looks fine now. But, adding a simpler example looks good if possible. Updated in b9407ca

HyukjinKwon · 2020-04-16T02:19:47Z

Nice, thanks you so much @maropu. I have some ideas on my mind for example, not including the generated MD files into the Github repo. We can of course include them but it might be better to do it separately because we're already not including md files in SQL built-in functions.

Do you mind if I manually push some changes into your branch (and to your PR)? Of course, feel free to edit as you want after that. My changes might be mostly just my preference or nits I guess.

maropu · 2020-04-16T02:28:29Z

Nice, thanks you so much @maropu. I have some ideas on my mind for example, not including the generated MD files into the Github repo. We can of course include them but it might be better to do it separately because we're already not including md files in SQL built-in functions.

Yea, the idea look reasonable.

Do you mind if I manually push some changes into your branch (and to your PR)? Of course, feel free to edit as you want after that. My changes might be mostly just my preference or nits I guess.

Of course not! Both is ok to me.

SparkQA · 2020-04-16T04:15:53Z

Test build #121346 has finished for PR 28224 at commit b9407ca.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-04-16T07:05:02Z

Test build #121350 has finished for PR 28224 at commit d66a9aa.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2020-04-16T07:27:07Z

retest this please

SparkQA · 2020-04-16T10:25:33Z

Test build #121355 has finished for PR 28224 at commit d66a9aa.

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

docs/_data/menu-sql.yaml

SparkQA · 2020-04-17T18:30:10Z

Test build #121416 has finished for PR 28224 at commit 7e9ebdf.

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

SparkQA · 2020-04-20T12:20:57Z

Test build #121515 has finished for PR 28224 at commit f58335d.

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

docs/.gitignore

sql/gen-sql-functions-docs.py

maropu · 2020-04-20T12:41:10Z

Looks pretty nice, thanks for the update, @HyukjinKwon ! LGTM except for two comment.

nchammas · 2020-04-20T15:31:38Z

Silly question: What's the relationship of the SQL function docs being added in this PR to the SQL function docs we already have here?

maropu · 2020-04-20T23:44:45Z

Have you checked the discussion in #28170 (comment) ? The document in api/sql just includes a full flatten list of the functions. So, for popular built-in functions (e.g., aggregate, array, and date/time functions), we're planning to provide the categorized document page only for them.

maropu · 2020-04-21T00:29:36Z

Updated for my two comments in f302c91. Could you check again for final sign-off? @HyukjinKwon

HyukjinKwon · 2020-04-21T01:53:48Z

Silly question: What's the relationship of the SQL function docs being added in this PR to the SQL function docs we already have here?

Actually .. I target to remove that SQL function docs in long run. For the short term, #28224 (comment) is true - we don't list all expressions yet.

HyukjinKwon

LGTM

HyukjinKwon · 2020-04-21T01:54:48Z

I am going to merge this because the documentation generation build is passed in Github Actions. There's nothing to check with PR builder.

HyukjinKwon · 2020-04-21T01:54:55Z

Merged to master and branch-3.0.

…ilt-in functions This PR intends to add a Python script to generates a SQL document for built-in functions and the document in SQL references. To make SQL references complete. Yes; ![a](https://user-images.githubusercontent.com/692303/79406712-c39e1b80-7fd2-11ea-8b85-9f9cbb6efed3.png) ![b](https://user-images.githubusercontent.com/692303/79320526-eb46a280-7f44-11ea-8639-90b1fb2b8848.png) ![c](https://user-images.githubusercontent.com/692303/79320707-3365c500-7f45-11ea-9984-69ffe800fb87.png) Manually checked and added tests. Closes #28224 from maropu/SPARK-31429. Lead-authored-by: Takeshi Yamamuro <[email protected]> Co-authored-by: HyukjinKwon <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit e42dbe7) Signed-off-by: HyukjinKwon <[email protected]>

HyukjinKwon · 2020-04-21T01:57:46Z

@huaxingao do you want to separate pages, and address the leftover comments at #28224 (comment)? Should be pretty straightforward to do it now.

SparkQA · 2020-04-21T03:19:55Z

Test build #121550 has finished for PR 28224 at commit f302c91.

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

cloud-fan · 2020-04-21T08:27:14Z

docs/sql-ref-functions-builtin.md

+### Aggregate Functions
+{% include_relative generated-agg-funcs-table.html %}
+#### Examples
+{% include_relative generated-agg-funcs-examples.html %}


So we put all the examples in one section?

Yea, we currently put examples per group in one section like this;

### Aggrgate Functions <A list of aggregate functions> #### Examples <All examples of the aggregate functions listed above> ### Window Functions <A list of aggregate functions> #### Examples <All examples of the window functions listed above> ...

I think it's better to put everything about one function (signature, desc, examples, etc.) together. Did we decide to do this due to technical difficulties?

We can do it. I just a bit rushed to merge this to keep this PR narrow-scoped as a base work.
Nothing is completely pinned at this moment.

As an easy way, we can refer other DBMSs documentations and mimic I guess.

I think so and we can improve the structure by follow-up activities. But, I'm not sure the improvement should be applied at the 3.0 release.

yea we can improve it in 3.1

ok, thanks, I'll file jira later.

filed: https://issues.apache.org/jira/browse/SPARK-31513

huaxingao · 2020-04-22T02:01:56Z

I just saw this

Do we need this scalar function sections in getting started page? We either need to delete this section or write a short description and link it to the built-in functions page.

maropu · 2020-04-22T02:09:52Z

How about Scala Functions -> Built-in Functions, then adding a short description and a link to sql-ref-functions.html#built-in-functions?

huaxingao · 2020-04-22T02:17:21Z

Sounds good. I will fix this.

HyukjinKwon · 2020-04-22T02:54:04Z

Thanks @huaxingao!

### What changes were proposed in this pull request? This PR intends to add a new test suite for `ExpressionInfo`. Major changes are as follows; - Added a new test suite named `ExpressionInfoSuite` - To improve test coverage, added a test for error handling in `ExpressionInfoSuite` - Moved the `ExpressionInfo`-related tests from `UDFSuite` to `ExpressionInfoSuite` - Moved the related tests from `SQLQuerySuite` to `ExpressionInfoSuite` - Added a comment in `ExpressionInfoSuite` (followup of #28224) ### Why are the changes needed? To improve test suites/coverage. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added tests. Closes #28308 from maropu/SPARK-31526. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>

### What changes were proposed in this pull request? This PR intends to add a new test suite for `ExpressionInfo`. Major changes are as follows; - Added a new test suite named `ExpressionInfoSuite` - To improve test coverage, added a test for error handling in `ExpressionInfoSuite` - Moved the `ExpressionInfo`-related tests from `UDFSuite` to `ExpressionInfoSuite` - Moved the related tests from `SQLQuerySuite` to `ExpressionInfoSuite` - Added a comment in `ExpressionInfoSuite` (followup of #28224) ### Why are the changes needed? To improve test suites/coverage. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added tests. Closes #28308 from maropu/SPARK-31526. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 42f496f) Signed-off-by: HyukjinKwon <[email protected]>

gatorsmile · 2020-04-28T21:43:36Z

Thank you, this looks great!

gatorsmile · 2020-05-05T04:29:10Z

docs/sql-ref-functions-builtin.md

+### JSON Functions
+{% include_relative generated-json-funcs-table.html %}
+#### Examples
+{% include_relative generated-agg-funcs-examples.html %}


generated-agg-funcs-examples.html -> generated-json-funcs-examples.html ?

yea... @huaxingao @dilipbiswal Could you include this fix in your open PRs? #28451
or #28433

Sure. I will fix this.

Thanks, @huaxingao

Oops, it was my mistake I guess. Thanks guys,

gatorsmile · 2020-10-12T15:40:13Z

So far, it just covers a subset. Could we add more categories to cover more built-in functions listed in https://spark.apache.org/docs/latest/api/sql/index.html? For your reference, Presto has the complete list https://prestodb.io/docs/current/functions.html cc @huaxingao @maropu

maropu · 2020-10-12T23:29:22Z

Filed in https://issues.apache.org/jira/browse/SPARK-33124
Do you have time to work on this? @tanelk (If nobody takes this, I'll do it later.)

tanelk · 2020-10-13T15:35:05Z

Yeah, I can take a look

probot-autolabeler bot added DOCS SQL labels Apr 15, 2020

maropu force-pushed the SPARK-31429 branch from fc9f2d5 to ef6391e Compare April 15, 2020 09:41

kiszk reviewed Apr 15, 2020

View reviewed changes

Fix

3382bf5

maropu force-pushed the SPARK-31429 branch from ef6391e to 3382bf5 Compare April 15, 2020 23:48

maropu force-pushed the SPARK-31429 branch from 0ab0477 to 3382bf5 Compare April 16, 2020 00:33

Fix

b9407ca

Fix

d66a9aa

maropu mentioned this pull request Apr 17, 2020

[SPARK-23903][SQL] Add support for date extract #21479

Closed

Address nits and add window_funcs

7e9ebdf

HyukjinKwon force-pushed the SPARK-31429 branch from 741c3ae to 7e9ebdf Compare April 17, 2020 15:10

HyukjinKwon reviewed Apr 17, 2020

View reviewed changes

docs/_data/menu-sql.yaml Show resolved Hide resolved

maropu commented Apr 20, 2020

View reviewed changes

docs/.gitignore Outdated Show resolved Hide resolved

sql/gen-sql-functions-docs.py Outdated Show resolved Hide resolved

Final cleanup

f302c91

HyukjinKwon approved these changes Apr 21, 2020

View reviewed changes

HyukjinKwon closed this in e42dbe7 Apr 21, 2020

cloud-fan reviewed Apr 21, 2020

View reviewed changes

maropu mentioned this pull request Apr 22, 2020

[SPARK-31349][SQL][DOCS] Document built-in aggregate functions in SQL Reference #28120

Closed

maropu mentioned this pull request Apr 23, 2020

[SPARK-31526][SQL][TESTS] Add a new test suite for ExpressionInfo #28308

Closed

gatorsmile reviewed May 5, 2020

View reviewed changes

nchammas mentioned this pull request Dec 18, 2023

[SPARK-46437][DOCS] Remove cruft from the built-in SQL functions documentation #44393

Closed

[SPARK-31429][SQL][DOC] Automatically generates a SQL document for built-in functions #28224

[SPARK-31429][SQL][DOC] Automatically generates a SQL document for built-in functions #28224

Uh oh!

Conversation

maropu commented Apr 15, 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!

maropu commented Apr 15, 2020

Uh oh!

SparkQA commented Apr 15, 2020

Uh oh!

SparkQA commented Apr 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu Apr 15, 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!

huaxingao commented Apr 15, 2020

Uh oh!

huaxingao commented Apr 15, 2020

Uh oh!

maropu commented Apr 16, 2020

Uh oh!

maropu commented Apr 16, 2020

Uh oh!

huaxingao commented Apr 16, 2020

Uh oh!

SparkQA commented Apr 16, 2020

Uh oh!

maropu commented Apr 16, 2020

Uh oh!

HyukjinKwon commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maropu commented Apr 16, 2020

Uh oh!

SparkQA commented Apr 16, 2020

Uh oh!

SparkQA commented Apr 16, 2020

Uh oh!

maropu commented Apr 16, 2020

Uh oh!

SparkQA commented Apr 16, 2020

Uh oh!

Uh oh!

SparkQA commented Apr 17, 2020

Uh oh!

SparkQA commented Apr 20, 2020

Uh oh!

Uh oh!

Uh oh!

maropu commented Apr 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nchammas commented Apr 20, 2020

Uh oh!

maropu commented Apr 20, 2020

Uh oh!

maropu commented Apr 21, 2020

Uh oh!

HyukjinKwon commented Apr 21, 2020

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Apr 21, 2020

Uh oh!

HyukjinKwon commented Apr 21, 2020

Uh oh!

HyukjinKwon commented Apr 21, 2020

Uh oh!

SparkQA commented Apr 21, 2020

Uh oh!

Choose a reason for hiding this comment

maropu commented Apr 15, 2020 •

edited

Loading

maropu Apr 15, 2020 •

edited

Loading

HyukjinKwon commented Apr 16, 2020 •

edited

Loading

maropu commented Apr 20, 2020 •

edited

Loading

maropu Apr 21, 2020 •

edited

Loading

maropu commented Apr 22, 2020 •

edited

Loading

maropu May 5, 2020 •

edited

Loading