Skip to content

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented May 14, 2018

The title summarizes the change.

@rxin
Copy link
Contributor Author

rxin commented May 14, 2018

cc @gatorsmile @HyukjinKwon

Copy link
Member

@gatorsmile gatorsmile left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented May 14, 2018

Test build #90567 has finished for PR 21318 at commit 83c191f.

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

@rxin
Copy link
Contributor Author

rxin commented May 14, 2018

Hm the failure doesn't look like it's caused by this PR. Do you guys know what's going on?

* a little bit more compile-time safety to make sure the function exists.
*
* Spark also includes more built-in functions that are less common and are not defined here.
* You can still access them (and all the functions defined here) using the [[functions.expr()]] API
Copy link
Member

Choose a reason for hiding this comment

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

[error] /home/jenkins/workspace/SparkPullRequestBuilder/sql/core/target/java/org/apache/spark/sql/functions.java:7: error: unexpected text
[error]  * You can still access them (and all the functions defined here) using the {@link functions.expr()} API
[error]                                                                             ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/sql/core/target/java/org/apache/spark/sql/functions.java:9: error: unexpected text
[error]  * the latest version of Spark at {@link https://spark.apache.org/docs/latest/api/sql/index.html}.
[error]                                   ^

Seems both links are the problem in Javadoc. Shall we just use `functions.expr() ` and leave the https://spark.apache.org/docs/latest/api/sql/index.html like without [[...]]?

* Spark also includes more built-in functions that are less common and are not defined here.
* You can still access them (and all the functions defined here) using the [[functions.expr()]] API
* and calling them through a SQL expression string. You can find the entire list of functions for
* the latest version of Spark at [[https://spark.apache.org/docs/latest/api/sql/index.html]].
Copy link
Member

@HyukjinKwon HyukjinKwon May 14, 2018

Choose a reason for hiding this comment

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

@rxin, it's rather a nit but shouldn't we always update the link for each release since it always points the latest? Probably, just prose is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's just a lot of work and i'm sure we will forget to update ... so i'm pointing to the latest.


/**
* Functions available for DataFrame operations.
* Commonly used functions available for DataFrame operations. Using functions defined here provides
Copy link
Member

@HyukjinKwon HyukjinKwon May 14, 2018

Choose a reason for hiding this comment

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

Maybe I am too much caring about this but I hope we don't have arguments too much about which function is common or not ...

* the latest version of Spark at [[https://spark.apache.org/docs/latest/api/sql/index.html]].
*
* As an example, `isnan` is a function that is defined here. You can use `isnan(col("myCol"))`
* to invoke the isnan function. This way the programming language's compiler ensures isnan exists
Copy link
Member

Choose a reason for hiding this comment

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

nit: isnan -> `isnan`

@HyukjinKwon
Copy link
Member

HyukjinKwon commented May 14, 2018

@rxin, should we maybe we mention that SQL functions are usually added to match other DBMSs (or Hive) (unlike functions.scala)?

* As an example, `isnan` is a function that is defined here. You can use `isnan(col("myCol"))`
* to invoke the isnan function. This way the programming language's compiler ensures isnan exists
* and is of the proper form. You can also use `expr("isnan(myCol)")` function to invoke the same
* function. In this case, Spark itself will ensure isnan exists when it analyzes the query.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: isnan

@gatorsmile
Copy link
Member

retest this please

@rxin
Copy link
Contributor Author

rxin commented May 15, 2018

It's still going to fail because I haven't updated it yet. Will do tomorrow.

@SparkQA
Copy link

SparkQA commented May 15, 2018

Test build #90625 has finished for PR 21318 at commit 83c191f.

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

@srowen
Copy link
Member

srowen commented Jul 27, 2018

Just browsing old PRs .. want to finish this one up @rxin? looks simple and useful.

@rxin
Copy link
Contributor Author

rxin commented Jul 27, 2018 via email

@SparkQA
Copy link

SparkQA commented Jul 28, 2018

Test build #93695 has finished for PR 21318 at commit db44914.

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

@gatorsmile
Copy link
Member

Thanks! Merged to master.

@asfgit asfgit closed this in 6424b14 Jul 28, 2018
@HyukjinKwon
Copy link
Member

@rxin re: #21318 (comment) I meant to say for instance, like "Please refer the SQL function documentation in the corresponding version". We don't have to bother update and also it makes sense ..

@rxin
Copy link
Contributor Author

rxin commented Jul 28, 2018 via email

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.

6 participants