[SPARK-31562][SQL] Update ExpressionDescription for substring, current_date, and current_timestamp #28342

maropu · 2020-04-25T08:22:00Z

What changes were proposed in this pull request?

This PR intends to add entries for substring, current_date, and current_timestamp in the SQL built-in function documents. Specifically, the entries are as follows;

SELECT current_date;
SELECT current_timestamp;
SELECT substring('abcd' FROM 1);
SELECT substring('abcd' FROM 1 FOR 2);

Why are the changes needed?

To make the SQL (built-in functions) references complete.

Does this PR introduce any user-facing change?

How was this patch tested?

Added test examples.

maropu · 2020-04-25T08:30:38Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

    expression[MonthsBetween]("months_between"),
    expression[NextDay]("next_day"),
-    expression[CurrentTimestamp]("now", true),
+    expression[Now]("now"),


Since the usage is different between now and current_timestamp, I added a Now expr.

btw, we don't need to add a version here where an alias added? For example, current_timestamp and now have been implemented in the different versions, 1.5 and 1.6.

What is the difference between now and current_timestamp? Should now extend Nondeterministic, for example?

CURRENT_TIMESTAMP is a SQL syntax and NOW is not: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4#L777

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

Test build #121801 has finished for PR 28342 at commit f4a61b8.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
abstract class CurrentTimestampLike() extends LeafExpression with CodegenFallback
case class CurrentTimestamp() extends CurrentTimestampLike
case class Now() extends CurrentTimestampLike

SparkQA · 2020-04-25T15:58:25Z

Test build #121806 has finished for PR 28342 at commit e77a8f8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
abstract class CurrentTimestampLike() extends LeafExpression with CodegenFallback
case class CurrentTimestamp() extends CurrentTimestampLike
case class Now() extends CurrentTimestampLike

dongjoon-hyun · 2020-04-25T16:44:40Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala

+@ExpressionDescription(
+  usage = "_FUNC_() - Returns the current timestamp at the start of query evaluation.",
+  group = "datetime_funcs",
+  since = "1.5.0")


I guess now should be 3.1.0.

Actually, now has been supported since 1.6.0? https://issues.apache.org/jira/browse/SPARK-11768
Or, you suggested above that we should set the version where this expr added?

Oh. Sorry, I was confused. Never mind~

maropu · 2020-04-25T22:51:16Z

cc: @HyukjinKwon @cloud-fan

HyukjinKwon · 2020-04-26T03:34:11Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala

+    Examples:
+      > SELECT _FUNC_();
+       2020-04-25
+      > SELECT _FUNC_;


Do you remember when we started to support this syntax? I roughly remember it was added relatively recently. If the added version is different, it might be best to note it in note while we're here.

I found the PR to implement the syntax: #14442
Spark without the hive support has supported it since 2.0. Yea, I'll add this info as note.

HyukjinKwon

Looks fine. one question.

dongjoon-hyun · 2020-04-26T05:06:22Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala

-  override def prettyName: String =
-    getTagValue(FunctionRegistry.FUNC_ALIAS).getOrElse("current_timestamp")
+@ExpressionDescription(
+  usage = "_FUNC_() - Returns the current timestamp at the start of query evaluation.",


nit. Do you think we can have examples = for now() since now usage is different(having different class and only functional form) from current_timestamp? Of course, we can skip it.

Ah I see. I'll add an example for now in this PR, too. Actually, I think it is better to add examples for the expressions that do not have examples now.

dongjoon-hyun · 2020-04-26T05:09:25Z

sql/gen-sql-functions-docs.py


    for info in infos:
        # Extracts (signature, description) pairs from `info.usage`, e.g.,
        # the signature is `func(expr)` and the description is `...` in an usage `func(expr) - ...`.


Do we need to update this description accordingly because this PR dropped ( and ) requirement?

dongjoon-hyun

+1, LGTM. (only minor nits which can be ignored.)

SparkQA · 2020-04-26T09:35:24Z

Test build #121834 has finished for PR 28342 at commit 463c4d3.

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

dongjoon-hyun

Thank you so much, @maropu and @HyukjinKwon .
Merged to master/3.0.

…t_date, and current_timestamp ### What changes were proposed in this pull request? This PR intends to add entries for substring, current_date, and current_timestamp in the SQL built-in function documents. Specifically, the entries are as follows; - SELECT current_date; - SELECT current_timestamp; - SELECT substring('abcd' FROM 1); - SELECT substring('abcd' FROM 1 FOR 2); ### Why are the changes needed? To make the SQL (built-in functions) references complete. ### Does this PR introduce any user-facing change? <img width="1040" alt="Screen Shot 2020-04-25 at 16 51 07" src="https://user-images.githubusercontent.com/692303/80274851-6ca5ee00-8718-11ea-9a35-9ae82008cb4b.png"> <img width="974" alt="Screen Shot 2020-04-25 at 17 24 24" src="https://user-images.githubusercontent.com/692303/80275032-a88d8300-8719-11ea-92ec-95b80169ae28.png"> <img width="862" alt="Screen Shot 2020-04-25 at 17 27 48" src="https://user-images.githubusercontent.com/692303/80275114-36696e00-871a-11ea-8e39-02e93eabb92f.png"> ### How was this patch tested? Added test examples. Closes #28342 from maropu/SPARK-31562. Authored-by: Takeshi Yamamuro <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit e01125d) Signed-off-by: Dongjoon Hyun <[email protected]>

maropu · 2020-04-27T00:11:12Z

Thanks, guys! @dongjoon-hyun @HyukjinKwon

probot-autolabeler bot added the SQL label Apr 25, 2020

maropu commented Apr 25, 2020

View reviewed changes

Fix

e77a8f8

maropu force-pushed the SPARK-31562 branch from f4a61b8 to e77a8f8 Compare April 25, 2020 12:48

dongjoon-hyun reviewed Apr 25, 2020

View reviewed changes

HyukjinKwon reviewed Apr 26, 2020

View reviewed changes

HyukjinKwon approved these changes Apr 26, 2020

View reviewed changes

dongjoon-hyun reviewed Apr 26, 2020

View reviewed changes

dongjoon-hyun approved these changes Apr 26, 2020

View reviewed changes

Fix

463c4d3

dongjoon-hyun approved these changes Apr 26, 2020

View reviewed changes

dongjoon-hyun closed this in e01125d Apr 26, 2020

[SPARK-31562][SQL] Update ExpressionDescription for substring, current_date, and current_timestamp #28342

[SPARK-31562][SQL] Update ExpressionDescription for substring, current_date, and current_timestamp #28342

Uh oh!

Conversation

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

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 25, 2020

Uh oh!

SparkQA commented Apr 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu commented Apr 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Apr 26, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 26, 2020

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

maropu commented Apr 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maropu commented Apr 25, 2020 •

edited

Loading

maropu Apr 26, 2020 •

edited

Loading

dongjoon-hyun Apr 26, 2020 •

edited

Loading