[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[mgaido91]

Github user mgaido91 closed the pull request at:

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


---

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

[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[mgaido91]

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

https://github.com/apache/spark/pull/19883#discussion_r155226705
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
 ---
@@ -444,11 +444,11 @@ case class DayOfWeek(child: Expression) extends 
UnaryExpression with ImplicitCas
   val cal = classOf[Calendar].getName
   val c = ctx.freshName("cal")
   val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
-  ctx.addMutableState(cal, c, s"""$c = 
$cal.getInstance($dtu.getTimeZone("UTC"));""")
--- End diff --

now I got it, sorry. I think the same thing can be said also for the 
timezone. Thus I am closing this. Sorry again.


---

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

[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19883#discussion_r155222959
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
 ---
@@ -444,11 +444,11 @@ case class DayOfWeek(child: Expression) extends 
UnaryExpression with ImplicitCas
   val cal = classOf[Calendar].getName
   val c = ctx.freshName("cal")
   val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
-  ctx.addMutableState(cal, c, s"""$c = 
$cal.getInstance($dtu.getTimeZone("UTC"));""")
--- End diff --

yea it's done for each expression, but each expression is evaluated many 
times.


---

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

[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[mgaido91]

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

https://github.com/apache/spark/pull/19883#discussion_r155221911
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
 ---
@@ -444,11 +444,11 @@ case class DayOfWeek(child: Expression) extends 
UnaryExpression with ImplicitCas
   val cal = classOf[Calendar].getName
   val c = ctx.freshName("cal")
   val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
-  ctx.addMutableState(cal, c, s"""$c = 
$cal.getInstance($dtu.getTimeZone("UTC"));""")
--- End diff --

but this is done every time, since we are crating a new one for each 
expression


---

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

[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/19883#discussion_r155221232
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
 ---
@@ -444,11 +444,11 @@ case class DayOfWeek(child: Expression) extends 
UnaryExpression with ImplicitCas
   val cal = classOf[Calendar].getName
   val c = ctx.freshName("cal")
   val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
-  ctx.addMutableState(cal, c, s"""$c = 
$cal.getInstance($dtu.getTimeZone("UTC"));""")
--- End diff --

this avoids calling `getInstance` every time.


---

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

[GitHub] spark pull request #19883: [SPARK-22684][SQL] datetime functions should not ...

[mgaido91]

GitHub user mgaido91 opened a pull request:

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

[SPARK-22684][SQL] datetime functions should not generate useless mutable 
states

## What changes were proposed in this pull request?

Some datetime functions are defining mutable states which are not needed at 
all. This is bad for the well known issues related to constant pool limits.

## How was this patch tested?

added UTs


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

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

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

https://github.com/apache/spark/pull/19883.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 #19883


commit 78850c6724f854ed317f22ee9845117fa9acae46
Author: Marco Gaido 
Date:   2017-12-04T21:34:00Z

[SPARK-22684][SQL] Avoid the generation of useless mutable states by 
datetime functions




---

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