[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/21008


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[mgaido91]

Github user mgaido91 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21008#discussion_r183067178
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala
 ---
@@ -453,34 +453,45 @@ class DateExpressionsSuite extends SparkFunSuite with 
ExpressionEvalHelper {
 MonthsBetween(
   Literal(new Timestamp(sdf.parse("1997-02-28 10:30:00").getTime)),
   Literal(new Timestamp(sdf.parse("1996-10-30 00:00:00").getTime)),
-  timeZoneId),
-3.94959677)
-  checkEvaluation(
-MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-01-30 11:52:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-01-30 11:50:00").getTime)),
-  timeZoneId),
-0.0)
+  Literal.TrueLiteral,
+  timeZoneId = timeZoneId), 3.94959677)
   checkEvaluation(
 MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-01-31 00:00:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-03-31 22:00:00").getTime)),
-  timeZoneId),
--2.0)
-  checkEvaluation(
-MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-03-31 22:00:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-02-28 00:00:00").getTime)),
-  timeZoneId),
-1.0)
+  Literal(new Timestamp(sdf.parse("1997-02-28 10:30:00").getTime)),
+  Literal(new Timestamp(sdf.parse("1996-10-30 00:00:00").getTime)),
+  Literal.FalseLiteral,
+  timeZoneId = timeZoneId), 3.9495967741935485)
+
+  Seq(Literal.FalseLiteral, Literal.TrueLiteral). foreach { roundOff =>
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-01-30 
11:52:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-01-30 
11:50:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), 0.0)
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-01-31 
00:00:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-03-31 
22:00:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), -2.0)
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-03-31 
22:00:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-02-28 
00:00:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), 1.0)
+  }
   val t = Literal(Timestamp.valueOf("2015-03-31 22:00:00"))
   val tnull = Literal.create(null, TimestampType)
-  checkEvaluation(MonthsBetween(t, tnull, timeZoneId), null)
-  checkEvaluation(MonthsBetween(tnull, t, timeZoneId), null)
-  checkEvaluation(MonthsBetween(tnull, tnull, timeZoneId), null)
+  checkEvaluation(MonthsBetween(t, tnull, Literal.TrueLiteral, 
timeZoneId = timeZoneId), null)
+  checkEvaluation(MonthsBetween(tnull, t, Literal.TrueLiteral, 
timeZoneId = timeZoneId), null)
+  checkEvaluation(
+MonthsBetween(tnull, tnull, Literal.TrueLiteral, timeZoneId = 
timeZoneId), null)
--- End diff --

it returns null, I added a test case for it, thanks.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[mgaido91]

Github user mgaido91 commented on a diff in the pull request:

https://github.com/apache/spark/pull/21008#discussion_r183065347
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
 ---
@@ -870,24 +870,14 @@ object DateTimeUtils {
* If time1 and time2 having the same day of month, or both are the last 
day of month,
* it returns an integer (time under a day will be ignored).
*
-   * Otherwise, the difference is calculated based on 31 days per month, 
and rounding to
-   * 8 digits.
+   * Otherwise, the difference is calculated based on 31 days per month.
+   * If `roundOff` is set to true, the result is rounded to 8 decimal 
places.
*/
-  def monthsBetween(time1: SQLTimestamp, time2: SQLTimestamp): Double = {
--- End diff --

yes, it was never used


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[ueshin]

Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/21008#discussion_r182625943
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
 ---
@@ -870,24 +870,14 @@ object DateTimeUtils {
* If time1 and time2 having the same day of month, or both are the last 
day of month,
* it returns an integer (time under a day will be ignored).
*
-   * Otherwise, the difference is calculated based on 31 days per month, 
and rounding to
-   * 8 digits.
+   * Otherwise, the difference is calculated based on 31 days per month.
+   * If `roundOff` is set to true, the result is rounded to 8 decimal 
places.
*/
-  def monthsBetween(time1: SQLTimestamp, time2: SQLTimestamp): Double = {
--- End diff --

Can we remove this?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[ueshin]

Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/21008#discussion_r182625871
  
--- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala
 ---
@@ -453,34 +453,45 @@ class DateExpressionsSuite extends SparkFunSuite with 
ExpressionEvalHelper {
 MonthsBetween(
   Literal(new Timestamp(sdf.parse("1997-02-28 10:30:00").getTime)),
   Literal(new Timestamp(sdf.parse("1996-10-30 00:00:00").getTime)),
-  timeZoneId),
-3.94959677)
-  checkEvaluation(
-MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-01-30 11:52:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-01-30 11:50:00").getTime)),
-  timeZoneId),
-0.0)
+  Literal.TrueLiteral,
+  timeZoneId = timeZoneId), 3.94959677)
   checkEvaluation(
 MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-01-31 00:00:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-03-31 22:00:00").getTime)),
-  timeZoneId),
--2.0)
-  checkEvaluation(
-MonthsBetween(
-  Literal(new Timestamp(sdf.parse("2015-03-31 22:00:00").getTime)),
-  Literal(new Timestamp(sdf.parse("2015-02-28 00:00:00").getTime)),
-  timeZoneId),
-1.0)
+  Literal(new Timestamp(sdf.parse("1997-02-28 10:30:00").getTime)),
+  Literal(new Timestamp(sdf.parse("1996-10-30 00:00:00").getTime)),
+  Literal.FalseLiteral,
+  timeZoneId = timeZoneId), 3.9495967741935485)
+
+  Seq(Literal.FalseLiteral, Literal.TrueLiteral). foreach { roundOff =>
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-01-30 
11:52:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-01-30 
11:50:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), 0.0)
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-01-31 
00:00:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-03-31 
22:00:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), -2.0)
+checkEvaluation(
+  MonthsBetween(
+Literal(new Timestamp(sdf.parse("2015-03-31 
22:00:00").getTime)),
+Literal(new Timestamp(sdf.parse("2015-02-28 
00:00:00").getTime)),
+roundOff,
+timeZoneId = timeZoneId), 1.0)
+  }
   val t = Literal(Timestamp.valueOf("2015-03-31 22:00:00"))
   val tnull = Literal.create(null, TimestampType)
-  checkEvaluation(MonthsBetween(t, tnull, timeZoneId), null)
-  checkEvaluation(MonthsBetween(tnull, t, timeZoneId), null)
-  checkEvaluation(MonthsBetween(tnull, tnull, timeZoneId), null)
+  checkEvaluation(MonthsBetween(t, tnull, Literal.TrueLiteral, 
timeZoneId = timeZoneId), null)
+  checkEvaluation(MonthsBetween(tnull, t, Literal.TrueLiteral, 
timeZoneId = timeZoneId), null)
+  checkEvaluation(
+MonthsBetween(tnull, tnull, Literal.TrueLiteral, timeZoneId = 
timeZoneId), null)
--- End diff --

What if `roundOff` is `null`?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #21008: [SPARK-23902][SQL] Add roundOff flag to months_be...

[mgaido91]

GitHub user mgaido91 opened a pull request:

https://github.com/apache/spark/pull/21008

[SPARK-23902][SQL] Add roundOff flag to months_between

## What changes were proposed in this pull request?

HIVE-15978 introduced the `roundOff` flag in order to disable the rounding 
to 8 digits which is performed in `months_between`. Since this can be a 
computational intensive operation, skipping it may improve performances when 
the rounding is not needed.

## How was this patch tested?

modified existing UT


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/mgaido91/spark SPARK-23902

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/21008.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #21008


commit 4544dd49968a0b8cd3e9c855575951447bfd2e24
Author: Marco Gaido 
Date:   2018-04-09T11:30:36Z

[SPARK-23902][SQL] Add roundOff flag to months_between




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org