Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

What changes were proposed in this pull request?

Currently, many functions do now show usages like the followings.

scala> sql("desc function extended `sin`").collect().foreach(println)
[Function: sin]
[Class: org.apache.spark.sql.catalyst.expressions.Sin]
[Usage: To be added.]
[Extended Usage:
To be added.]

This PR adds descriptions for functions and adds a testcase prevent adding function without usage.

scala>  sql("desc function extended `sin`").collect().foreach(println);
[Function: sin]
[Class: org.apache.spark.sql.catalyst.expressions.Sin]
[Usage: sin(x) - Returns the sine of x.]
[Extended Usage:
> SELECT sin(0);
 0.0]

The only exceptions are cube, grouping, grouping_id, rollup, window.

How was this patch tested?

Pass the Jenkins tests (including new testcases.)

@SparkQA
Copy link

SparkQA commented Apr 5, 2016

Test build #55030 has finished for PR 12185 at commit 1bc40e1.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-14415] Add ExpressionDescription annotation for SQL expressions [SPARK-14415][SQL] Add ExpressionDescription annotation for SQL expressions Apr 5, 2016
@SparkQA
Copy link

SparkQA commented Apr 6, 2016

Test build #55039 has finished for PR 12185 at commit 26a1865.

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

@SparkQA
Copy link

SparkQA commented Apr 6, 2016

Test build #55084 has finished for PR 12185 at commit 2d7cee8.

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

@SparkQA
Copy link

SparkQA commented Apr 7, 2016

Test build #55176 has finished for PR 12185 at commit 171389e.

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

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-14415][SQL] Add ExpressionDescription annotation for SQL expressions [SPARK-14415][SQL] All functions should show usages by command DESC FUNCTION Apr 8, 2016
@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55283 has finished for PR 12185 at commit b1d5cd1.

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

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55304 has finished for PR 12185 at commit e41326b.

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

@SparkQA
Copy link

SparkQA commented Apr 8, 2016

Test build #55313 has finished for PR 12185 at commit 68c90a0.

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

@dongjoon-hyun
Copy link
Member Author

Hi, @rxin .
Could you give me some directional advice to improve this PR?

@rxin
Copy link
Contributor

rxin commented Apr 9, 2016

This is super useful.

@yhuai can you review this?

@dongjoon-hyun
Copy link
Member Author

Thank you so much! @rxin :)

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by (array, value)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Returns the average of values in a group?

@dongjoon-hyun
Copy link
Member Author

Oh, @yhuai . Thank you for deep and detail review!
I'll fix all of them and improve as possible as I can.

@yhuai
Copy link
Contributor

yhuai commented Apr 9, 2016

For those aggregate functions, I feel it is better to mention that they are calculated from values of a group instead of just saying from a set of values?

@dongjoon-hyun
Copy link
Member Author

Sure. You mean
'Returns the XXX of a set of numbers' -> 'Returns the XXX calculated from values of a group'
Right?

@dongjoon-hyun
Copy link
Member Author

Since I just borrowed from the Hive cli, it seems to be weird. I think we can improve them in this PR as much as possible.

@yhuai
Copy link
Contributor

yhuai commented Apr 9, 2016

Yea. I feel that calculated from values of a group is a little bit better.

@dongjoon-hyun
Copy link
Member Author

Thank you. I agree. I will apply them all related descriptions.

@SparkQA
Copy link

SparkQA commented Apr 9, 2016

Test build #55454 has finished for PR 12185 at commit 03d3395.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55455 has finished for PR 12185 at commit 080891f.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55467 has finished for PR 12185 at commit d88121c.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55468 has finished for PR 12185 at commit ff20b07.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2016

Test build #55477 has finished for PR 12185 at commit d0f5ed6.

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

@yhuai
Copy link
Contributor

yhuai commented Apr 10, 2016

Thanks. Merging to master.

@asfgit asfgit closed this in a7ce473 Apr 10, 2016
@dongjoon-hyun
Copy link
Member Author

Thank you, @yhuai and @rxin .

asfgit pushed a commit that referenced this pull request Apr 19, 2016
…ection Functions

#### What changes were proposed in this pull request?
#12185 contains the original PR I submitted in #10418

However, it misses one of the extended example, a wrong description and a few typos for collection functions. This PR is fix all these issues.

#### How was this patch tested?
The existing test cases already cover it.

Author: gatorsmile <[email protected]>

Closes #12492 from gatorsmile/expressionUpdate.
@dongjoon-hyun dongjoon-hyun deleted the SPARK-14415 branch May 12, 2016 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants