[SPARK-28107][SQL] Support 'DAY TO (HOUR|MINUTE|SECOND)', 'HOUR TO (MINUTE|SECOND)' and 'MINUTE TO SECOND' #25000

zhulipeng · 2019-06-28T09:40:33Z

What changes were proposed in this pull request?

The interval conversion behavior is same with the PostgreSQL.

https://github.com/postgres/postgres/blob/REL_12_BETA2/src/test/regress/sql/interval.sql#L180-L203

How was this patch tested?

UT.

…o second

HyukjinKwon · 2019-06-28T12:32:14Z

ok to test

SparkQA · 2019-06-28T14:51:08Z

Test build #107004 has finished for PR 25000 at commit 8c99767.

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

dongjoon-hyun · 2019-07-03T04:08:13Z

Retest this please.

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

dongjoon-hyun · 2019-07-03T04:11:18Z

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

+            seconds = m.group(6) == null ? toLongWithRange("minute", m.group(5), 0, 59)
+              : toLongWithRange("second", m.group(7), 0, 59);
+            break;
+          default:


If we have only two cases, shall we use if ... else instead of switch?

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

SparkQA · 2019-07-03T05:56:29Z

Test build #107150 has finished for PR 25000 at commit 8c99767.

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

zhulipeng · 2019-07-03T11:38:56Z

@HyukjinKwon Thanks for your comments, just did some updates according to your suggestions.

SparkQA · 2019-07-03T11:47:06Z

Test build #107179 has finished for PR 25000 at commit ed4b3cf.

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

dongjoon-hyun · 2019-07-03T16:42:09Z

Retest this please.

SparkQA · 2019-07-03T16:45:14Z

Test build #107190 has finished for PR 25000 at commit ed4b3cf.

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

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

SparkQA · 2019-07-04T03:41:12Z

Test build #107208 has finished for PR 25000 at commit 3edfed6.

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

SparkQA · 2019-07-04T05:45:58Z

Test build #107216 has finished for PR 25000 at commit fa68a41.

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

dongjoon-hyun · 2019-07-05T07:54:27Z

Retest this please.

SparkQA · 2019-07-05T10:24:46Z

Test build #107266 has finished for PR 25000 at commit fa68a41.

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

dongjoon-hyun · 2019-07-05T22:56:56Z

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

   */
-  public static CalendarInterval fromDayTimeString(String s) throws IllegalArgumentException {
-    CalendarInterval result = null;
+  public static CalendarInterval fromDayTimeString(String s) {


We still need throws IllegalArgumentException here.

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

dongjoon-hyun · 2019-07-05T23:30:14Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

+        "899 microseconds")))
+    checkAnswer(sql("select interval '40:32.99899999' minute to second"),
+      Row(CalendarInterval.fromString("interval 40 minutes 32 seconds 99 milliseconds " +
+        "899 microseconds")))


Line 1203 ~ 1205 are the same with line 1206 ~ 1208.

dongjoon-hyun · 2019-07-05T23:34:10Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

    checkAnswer(sql("select interval '10-9' year to month"),
      Row(CalendarInterval.fromString("interval 10 years 9 months")))
+    checkAnswer(sql("select interval '20 15:40:32.99899999' day to hour"),
+        Row(CalendarInterval.fromString("interval 2 weeks 6 days 15 hours")))


indentation?

SparkQA · 2019-07-09T07:05:02Z

Test build #107393 has finished for PR 25000 at commit c41dccc.

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

dongjoon-hyun · 2019-07-09T15:42:35Z

Retest this please.

SparkQA · 2019-07-09T18:11:21Z

Test build #107409 has finished for PR 25000 at commit c41dccc.

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

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

SparkQA · 2019-07-10T04:41:42Z

Test build #107424 has finished for PR 25000 at commit d68ea26.

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

dongjoon-hyun · 2019-07-10T04:50:28Z

Hi, @lipzhu . I made a PR to your branch. Could you review and merge that?

https://github.com/lipzhu/spark/pull/3

It's just for minimizing and simplifying the diff of the patch. I checked that all newly added UT passed.

minimize

dongjoon-hyun · 2019-07-10T04:55:04Z

Thank you for merging.

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

SparkQA · 2019-07-10T07:05:02Z

Test build #107436 has finished for PR 25000 at commit bf78d73.

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

SparkQA · 2019-07-10T07:05:02Z

Test build #107438 has finished for PR 25000 at commit ebe917c.

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

dongjoon-hyun · 2019-07-10T14:20:49Z

Retest this please.

SparkQA · 2019-07-10T16:18:24Z

Test build #107461 has finished for PR 25000 at commit ebe917c.

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

dongjoon-hyun · 2019-07-10T16:43:14Z

Retest this please

SparkQA · 2019-07-10T18:54:24Z

Test build #107471 has finished for PR 25000 at commit ebe917c.

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

dongjoon-hyun · 2019-07-10T19:44:37Z

The first failure is the following.

org.apache.spark.sql.streaming.continuous.ContinuousStressSuite.restarts

The second failure is the following.

org.apache.spark.scheduler.TaskSchedulerImplSuite.SPARK-22148 abort timer should clear unschedulableTaskSetToExpiryTime for all TaskSets

Both are irrelevant to this PR.

dongjoon-hyun · 2019-07-10T19:44:44Z

Retest this please

SparkQA · 2019-07-10T22:45:31Z

Test build #107482 has finished for PR 25000 at commit ebe917c.

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

dongjoon-hyun

+1, LGTM. Merged to master.
Thank you so much, @lipzhu !

zhulipeng · 2019-07-11T01:54:29Z

@dongjoon-hyun Thanks for your patient and work on this.

gatorsmile · 2019-07-21T05:24:58Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

      Row(CalendarInterval.fromString("interval 100 milliseconds")))
    checkAnswer(sql("select interval '10-9' year to month"),
      Row(CalendarInterval.fromString("interval 10 years 9 months")))
+    checkAnswer(sql("select interval '20 15:40:32.99899999' day to hour"),


All these test cases are positive ones. Could you add the negative cases too? Do they throw an exception? Exception messages make sense?

support day to hour and day to minute and hour to minute and minute t…

8c99767

…o second

zhulipeng changed the title ~~[SQL]Support 'day to hour', 'day to minute', 'hour to minute' and 'minute to second'~~ [SPARK-28107][SQL]Support 'day to hour', 'day to minute', 'hour to minute' and 'minute to second' Jun 28, 2019

dongjoon-hyun added the SQL label Jun 28, 2019

dongjoon-hyun changed the title ~~[SPARK-28107][SQL]Support 'day to hour', 'day to minute', 'hour to minute' and 'minute to second'~~ [SPARK-28107][SQL] Support 'day to hour', 'day to minute', 'hour to minute' and 'minute to second' Jul 3, 2019

dongjoon-hyun reviewed Jul 3, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jul 3, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jul 3, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Show resolved Hide resolved

refine code

ed4b3cf

dongjoon-hyun reviewed Jul 3, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

length exceed

3edfed6

dongjoon-hyun reviewed Jul 4, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Outdated Show resolved Hide resolved

dongjoon-hyun reviewed Jul 4, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Outdated Show resolved Hide resolved

add test case for default branch

fa68a41

dongjoon-hyun reviewed Jul 5, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Show resolved Hide resolved

dongjoon-hyun reviewed Jul 5, 2019

View reviewed changes

zhulipeng changed the title ~~[SPARK-28107][SQL] Support 'day to hour', 'day to minute', 'hour to minute' and 'minute to second'~~ [SPARK-28107][SQL] Support 'day to hour', 'day to minute', ‘day to seconnd’, 'hour to minute', 'hour to second' and 'minute to second' Jul 9, 2019

for consistent

d68ea26

zhulipeng commented Jul 10, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Show resolved Hide resolved

minimize

7fd926a

dongjoon-hyun changed the title ~~[SPARK-28107][SQL] Support 'day to hour', 'day to minute', ‘day to second’, 'hour to minute', 'hour to second' and 'minute to second'~~ [SPARK-28107][SQL] Support 'DAY TO (HOUR|MINUTE|SECOND)', 'HOUR TO (MINUTE|SECOND)' and 'MINUTE TO SECOND' Jul 10, 2019

Merge pull request #3 from dongjoon-hyun/PR-25000

bf78d73

minimize

dongjoon-hyun reviewed Jul 10, 2019

View reviewed changes

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Outdated Show resolved Hide resolved

typo

ebe917c

dongjoon-hyun approved these changes Jul 11, 2019

View reviewed changes

dongjoon-hyun closed this in d26642d Jul 11, 2019

gatorsmile reviewed Jul 21, 2019

View reviewed changes

[SPARK-28107][SQL] Support 'DAY TO (HOUR|MINUTE|SECOND)', 'HOUR TO (MINUTE|SECOND)' and 'MINUTE TO SECOND' #25000

[SPARK-28107][SQL] Support 'DAY TO (HOUR|MINUTE|SECOND)', 'HOUR TO (MINUTE|SECOND)' and 'MINUTE TO SECOND' #25000

Uh oh!

Conversation

zhulipeng commented Jun 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HyukjinKwon commented Jun 28, 2019

Uh oh!

SparkQA commented Jun 28, 2019

Uh oh!

dongjoon-hyun commented Jul 3, 2019

Uh oh!

Uh oh!

dongjoon-hyun Jul 3, 2019 • 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!

Uh oh!

Uh oh!

SparkQA commented Jul 3, 2019

Uh oh!

zhulipeng commented Jul 3, 2019

Uh oh!

SparkQA commented Jul 3, 2019

Uh oh!

dongjoon-hyun commented Jul 3, 2019

Uh oh!

SparkQA commented Jul 3, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jul 4, 2019

Uh oh!

SparkQA commented Jul 4, 2019

Uh oh!

dongjoon-hyun commented Jul 5, 2019

Uh oh!

SparkQA commented Jul 5, 2019

Uh oh!

dongjoon-hyun Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dongjoon-hyun Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 9, 2019

Uh oh!

dongjoon-hyun commented Jul 9, 2019

Uh oh!

SparkQA commented Jul 9, 2019

Uh oh!

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Jul 10, 2019

Uh oh!

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 10, 2019

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 10, 2019

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

zhulipeng commented Jun 28, 2019 •

edited

Loading

dongjoon-hyun Jul 3, 2019 •

edited

Loading

dongjoon-hyun commented Jul 10, 2019 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading