[SPARK-16609] Add to_date/to_timestamp with format functions #16138

bllchmbrs · 2016-12-04T20:22:57Z

What changes were proposed in this pull request?

This pull request adds two new user facing functions:

to_date which accepts an expression and a format and returns a date.
to_timestamp which accepts an expression and a format and returns a timestamp.

For example, Given a date in format: 2016-21-05. (YYYY-dd-MM)

Date Function

Previously

to_date(unix_timestamp(lit("2016-21-05"), "yyyy-dd-MM").cast("timestamp"))

Current

to_date(lit("2016-21-05"), "yyyy-dd-MM")

Timestamp Function

Previously

unix_timestamp(lit("2016-21-05"), "yyyy-dd-MM").cast("timestamp")

Current

to_timestamp(lit("2016-21-05"), "yyyy-dd-MM")

Tasks

Add to_date to Scala Functions
Add to_date to Python Functions
Add to_date to SQL Functions
Add to_timestamp to Scala Functions
Add to_timestamp to Python Functions
Add to_timestamp to SQL Functions
Add function to R

How was this patch tested?

Add Functions to DateFunctionsSuite
Test new ParseToTimestamp Expression (not necessary)
Test new ParseToDate Expression (not necessary)
Add test for R
Add test for Python in test.py

Please review http://spark.apache.org/contributing.html before opening a pull request.

…8424

…6609

bllchmbrs · 2016-12-04T20:23:40Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

@@ -2666,7 +2666,18 @@ object functions {
   * @group datetime_funcs
   * @since 1.5.0
   */
-  def to_date(e: Column): Column = withExpr { ToDate(e.expr) }
+  def to_date(e: Column): Column = withExpr { ToDate(e.expr, Literal("yyyy-MM-dd")) }


this maintains previous capabilities.

bllchmbrs · 2016-12-04T20:24:58Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+   * (see [http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html])
+   *
+   * @group datetime_funcs
+   * @since 2.2.0


Will need to make sure this version is correct.

bllchmbrs · 2016-12-04T20:25:49Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+    * (see [http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html])
+    * to Unix time stamp (in seconds), return null if fail.
+    * @group datetime_funcs
+    * @since 1.5.0


will need to change this version to the correct branch.

SparkQA · 2016-12-04T20:31:02Z

Test build #69645 has finished for PR 16138 at commit b0f1ced.

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

rxin · 2016-12-04T20:36:53Z

Can you use "SPARK-16609" instead of "Spark-16609"?

bllchmbrs · 2016-12-04T20:54:56Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+    * @group datetime_funcs
+    * @since 1.5.0
+    */
+  def to_timestamp(s: Column, p: String): Column = withExpr {UnixTimestamp(s.expr, Literal(p)) }


still missing a cast here...

SparkQA · 2016-12-04T20:59:05Z

Test build #69647 has finished for PR 16138 at commit 9a10a34.

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

…6609

…it expressions

SparkQA · 2016-12-04T22:34:43Z

Test build #69648 has finished for PR 16138 at commit ecde7b7.

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

bllchmbrs · 2016-12-04T22:59:48Z

sql/core/src/test/scala/org/apache/spark/sql/DateFunctionsSuite.scala

+        Row(Date.valueOf("2014-12-31"))))
+    checkAnswer(
+      df.select(to_date(col("s"), "yyyy-MM-dd")),
+      Seq(Row(Date.valueOf("2015-07-22")), Row(Date.valueOf("2014-12-31")), Row(null)))


scala> val format = new java.text.SimpleDateFormat("yyyy-MM-dd") format: java.text.SimpleDateFormat = java.text.SimpleDateFormat@f67a0200 scala> format.parse("2014-31-12") res1: java.util.Date = Tue Jul 12 00:00:00 PDT 2016

This is strange to me but is expected behavior.

It's strange that this test does not pass on my local machine. Is there something that I am missing that is causing this?

nevermind, this does fail on here too. Seems like I was just looking at the wrong result.

SparkQA · 2016-12-04T23:48:54Z

Test build #69649 has finished for PR 16138 at commit 89dccbd.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ToDate(child: Expression) extends UnaryExpression with ImplicitCastInputTypes

SparkQA · 2016-12-05T00:19:19Z

Test build #69650 has finished for PR 16138 at commit c17c0bd.

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

cloud-fan · 2017-01-26T11:32:22Z

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

+  }
+
+  override def flatArguments: Iterator[Any] = Iterator(left, format)
+  override def sql: String = s"$prettyName(${left.sql}, ${format.sql})"


This is the problem: you set format to empty string at https://github.com/apache/spark/pull/16138/files#diff-b83497f7bc11578a0b63a814a2a30f48R1072 , but this is not safe. Although the format will be ignored, we use it to build the sql string of this expression, which maybe something like to_date('2012-12-12 12:12:12', '').

we should make format an Option[String], and generate corrected sql string here.

SparkQA · 2017-02-03T06:43:36Z

Test build #72301 has started for PR 16138 at commit a2d0221.

cloud-fan · 2017-02-03T10:15:55Z

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

+}
+
+  override def flatArguments: Iterator[Any] = Iterator(left, format)
+  override def sql: String = s"$prettyName(${left.sql}, ${format.sql})"


does it have the same issue as ParseToDate?

No, to_timestamp doesn't exist at all in hive, it's a brand new Spark SQL only function. I also can't actually find a failure in the test results. Do you know what actually failed?

hvanhovell · 2017-02-03T16:42:18Z

retest this please

SparkQA · 2017-02-03T18:55:54Z

Test build #72311 has finished for PR 16138 at commit a2d0221.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ParseToDate(left: Expression, format: Option[Expression], child: Expression)

SparkQA · 2017-02-04T22:12:09Z

Test build #72394 has finished for PR 16138 at commit 479243f.

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

SparkQA · 2017-02-04T22:19:34Z

Test build #72396 has finished for PR 16138 at commit e00b446.

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

SparkQA · 2017-02-05T02:13:04Z

Test build #72397 has finished for PR 16138 at commit da16c66.

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

felixcheung · 2017-02-05T05:49:51Z

this is the CRAN check error:

* checking for missing documentation entries ... WARNING
Undocumented S4 methods:
  generic 'to_date' and siglist 'Column,missing'
  generic 'to_timestamp' and siglist 'Column,missing'
All user-level objects in a package (including S4 classes and methods)
should have documentation entries.
See the chapter 'Writing R documentation files' in the 'Writing R
Extensions' manual.
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... WARNING
Objects in \usage without \alias in documentation object 'to_date':
  '\S4method{to_date}{Column,missing}'

Objects in \usage without \alias in documentation object 'to_timestamp':
  '\S4method{to_timestamp}{Column,missing}'

Functions with \usage entries need to have the appropriate \alias
entries, and all their arguments documented.
The \usage entries must correspond to syntactically valid R code.
See the chapter 'Writing R documentation files' in the 'Writing R

you probably change it to @aliases to_date,Column,missing-method

bllchmbrs · 2017-02-06T04:50:27Z

@felixcheung thanks, made those changes :). Hopefully this will start passing sometime :P

cloud-fan · 2017-02-06T07:12:55Z

sql/core/src/test/scala/org/apache/spark/sql/DateFunctionsSuite.scala

+    //  now switch format
+    checkAnswer(
+      df.select(to_date(col("s"), "yyyy-dd-MM")),
+      Seq(Row(Date.valueOf("2016-10-07")), Row(Date.valueOf("2016-07-12")),


I'm surprised, converting 2015-07-22 10:00:00 to date with format yyyy-dd-MM will result to 2016-10-07? When we convert 2014-31-12 to date with default format, it returns null.

SparkQA · 2017-02-06T07:23:41Z

Test build #72422 has finished for PR 16138 at commit c3063a2.

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

bllchmbrs · 2017-02-07T13:21:27Z

@cloud-fan great questions.

I thought that was strange too. However this is the current behavior as well as Java SimpleDateFormat's behavior. I did not implement that logic.

For example, today:

spark.sql("""
   SELECT 
   cast(
	   cast(
	   	unix_timestamp('2016-30-12', 'yyyy-MM-dd') 
	   as timestamp) 
   as date)
   """).take(5)
// res4: Array[org.apache.spark.sql.Row] = Array([2018-06-12])

Which seems wrong to me, but that's the logic that exists. The issue seems to be that unix_timestamp returns an incorrect value when it should just return null. The source is the behavior of SimpleDateFormat.

scala> import java.text.SimpleDateFormat
// import java.text.SimpleDateFormat

scala> val x = new SimpleDateFormat("yyyy-MM-dd")
// x: java.text.SimpleDateFormat = java.text.SimpleDateFormat@f67a0200

scala> x.parse("2016-30-12")
// res5: java.util.Date = Tue Jun 12 00:00:00 PDT 2018

bllchmbrs · 2017-02-07T13:21:53Z

Manual Correctness tests:

Python

>>> from pyspark.sql.functions import to_date, to_timestamp, lit
>>> spark.range(1).select(to_date(lit('2016-01-02'),'yyyy-MM-dd').alias("dt")).collect()
# [Row(dt=datetime.date(2016, 1, 2))]
>>> spark.range(1).select(to_timestamp(lit('2016-01-02'),'yyyy-MM-dd').alias("dt")).collect()
# [Row(dt=datetime.datetime(2016, 1, 2, 0, 0))]

R

> collect(select(people, to_date(lit('2016-01-02'), 'yyyy-MM-dd')))
  to_date('2016-01-02', 'yyyy-MM-dd'
#1                         2016-01-02
#2                         2016-01-02
#3                         2016-01-02
> collect(select(people, to_timestamp(lit('2016-01-02'), 'yyyy-MM-dd')))
  to_timestamp('2016-01-02', 'yyyy-MM-dd')
#1                               2016-01-02
#2                               2016-01-02
#3                               2016-01-02
> select(people, to_timestamp(lit('2016-01-02'), 'yyyy-MM-dd'))

cloud-fan · 2017-02-07T13:31:21Z

Since this is an existing behavior, this PR LGTM

hvanhovell · 2017-02-07T14:49:51Z

LGTM - merging master.

## What changes were proposed in this pull request? This pull request adds two new user facing functions: - `to_date` which accepts an expression and a format and returns a date. - `to_timestamp` which accepts an expression and a format and returns a timestamp. For example, Given a date in format: `2016-21-05`. (YYYY-dd-MM) ### Date Function *Previously* ``` to_date(unix_timestamp(lit("2016-21-05"), "yyyy-dd-MM").cast("timestamp")) ``` *Current* ``` to_date(lit("2016-21-05"), "yyyy-dd-MM") ``` ### Timestamp Function *Previously* ``` unix_timestamp(lit("2016-21-05"), "yyyy-dd-MM").cast("timestamp") ``` *Current* ``` to_timestamp(lit("2016-21-05"), "yyyy-dd-MM") ``` ### Tasks - [X] Add `to_date` to Scala Functions - [x] Add `to_date` to Python Functions - [x] Add `to_date` to SQL Functions - [X] Add `to_timestamp` to Scala Functions - [x] Add `to_timestamp` to Python Functions - [x] Add `to_timestamp` to SQL Functions - [x] Add function to R ## How was this patch tested? - [x] Add Functions to `DateFunctionsSuite` - Test new `ParseToTimestamp` Expression (*not necessary*) - Test new `ParseToDate` Expression (*not necessary*) - [x] Add test for R - [x] Add test for Python in test.py Please review http://spark.apache.org/contributing.html before opening a pull request. Author: anabranch <wac.chambers@gmail.com> Author: Bill Chambers <bill@databricks.com> Author: anabranch <bill@databricks.com> Closes apache#16138 from anabranch/SPARK-16609.

YannByron · 2017-08-25T07:22:26Z

@anabranch

I met a failure case when using to_date, like this:

 spark-sql> select func(to_date('2017-08-25 12:00:00'));

func is a custom udf which extends org.apache.hadoop.hive.ql.udf.generic.GenericUDF.

And this following exception happened:

Error in query: No handler for Hive UDF 'xxx.xxx.func': java.lang.UnsupportedOperationException: Cannot evaluate expression: to_date(2017-08-25 12:00:00, None);

I locate this problem. When resolving function func(to_date('2017-08-25 12:00:00'))(at this time Spark has already finished the resolving to func's child to_date('2017-08-25 12:00:00')), it will call func's FunctionBuilder that is defined in HiveSessionCatalog.makeFunctionBuilder).
In it, the GenericUDF instance will be forced to check input data type. During this process, All of the child catalyst expression which are the parameters of the subclass of GenericUDF, need to be mapped to ObjectInspector(see details: HiveInspector.toInspector). Inside, the child expression need to be evaluated though eval() firstly, if expression doesn't match with Literal.

def toInspector(expr: Expression): ObjectInspector = expr match {
    case Literal(value, XXXType) =>
        //...
    // ideally, we don't test the foldable here(but in optimizer), however, some of the
    // Hive UDF / UDAF requires its argument to be constant objectinspector, we do it eagerly.
    case _ if expr.foldable => toInspector(Literal.create(expr.eval(), expr.dataType))
}

In my case, the child expression is to_date('2017-08-25 12:00:00') that is the instance of ParseToDate (extends RuntimeReplaceable). When calling its eval(), we call the Unevaluable's eval() that will throw this exception mentioned above.

trait Unevaluable extends Expression {

  final override def eval(input: InternalRow = null): Any =
    throw new UnsupportedOperationException(s"Cannot evaluate expression: $this")

  final override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode =
    throw new UnsupportedOperationException(s"Cannot evaluate expression: $this")
}

Considering that we barely use the second parameter pattern, I remap to_date to ToDate again to fix the problem.

So, does my case relate to this PR?

bllchmbrs · 2017-08-30T02:32:22Z

I don't think this is necessarily my call but you're effectively writing a hive udf, not a Spark one that depends on this. Writing a Spark UDF would allow this to work just fine and the expression to be evaluated. Your work around seems fine in your local version though.

…

On Fri, Aug 25, 2017 at 12:23 AM, Yann Byron ***@***.***> wrote: @anabranch <https://github.com/anabranch> I met a failure case when using to_date, like this: spark-sql> select func(to_date('2017-08-25 12:00:00')); func is a custom udf which extends org.apache.hadoop.hive.ql.udf. generic.GenericUDF. And this following exception happened: Error in query: No handler for Hive UDF 'xxx.xxx.func': java.lang.UnsupportedOperationException: Cannot evaluate expression: to_date(2017-08-25 12:00:00, None); I locate this problem. When resolving function func(to_date('2017-08-25 12:00:00'))(at this time Spark has already finished the resolving to func's child to_date('2017-08-25 12:00:00')), it will call func's FunctionBuilder that is defined in HiveSessionCatalog.makeFunctionBuilder <https://github.com/apache/spark/blob/v2.2.0/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala> ). In it, the GenericUDF instance will be forced to check input data type. During this process, All of the child catalyst expression which are the parameters of the subclass of GenericUDF, need to be mapped to ObjectInspector(see details: HiveInspector.toInspector <https://github.com/apache/spark/blob/v2.2.0/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveInspectors.scala>). Inside, the child expression need to be evaluated though eval() firstly, if expression doesn't match with Literal. def toInspector(expr: Expression): ObjectInspector = expr match { case Literal(value, XXXType) => //... // ideally, we don't test the foldable here(but in optimizer), however, some of the // Hive UDF / UDAF requires its argument to be constant objectinspector, we do it eagerly. case _ if expr.foldable => toInspector(Literal.create(expr.eval(), expr.dataType)) } In my case, the child expression is to_date('2017-08-25 12:00:00') that is the instance of ParseToDate <https://github.com/apache/spark/blob/v2.2.0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala> (extends RuntimeReplaceable <https://github.com/apache/spark/blob/v2.2.0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala>). When calling its eval(), we call the Unevaluable <https://github.com/apache/spark/blob/v2.2.0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala>'s eval() that will throw this exception mentioned above. trait Unevaluable extends Expression { final override def eval(input: InternalRow = null): Any = throw new UnsupportedOperationException(s"Cannot evaluate expression: $this") final override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = throw new UnsupportedOperationException(s"Cannot evaluate expression: $this") } Considering that we barely use the second parameter pattern, I remap to_date to ToDate again to fix the problem. So, does my case relate to this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16138 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABkQBwa1lkNT81uAWDV3ZRSRz8XqApPHks5sbnaMgaJpZM4LDrz0> .

bllchmbrs added 9 commits November 15, 2016 19:18

prepping for adding the function

087feae

added base structure

bef59b1

time to start fixing tests

9db65b8

base function

feafa3d

Merge branch 'master' of https://github.com/apache/spark into SPARK-1…

562df34

…8424

improved comment

c4d997b

Merge branch 'master' of https://github.com/apache/spark into SPARK-1…

65d668f

…6609

first pass at code gen

fcf73ec

ready to start a discussion

27e4abe

bllchmbrs commented Dec 4, 2016

View reviewed changes

renaming

d0f81a3

bllchmbrs commented Dec 4, 2016

View reviewed changes

formatting

b0f1ced

bllchmbrs commented Dec 4, 2016

View reviewed changes

bllchmbrs changed the title ~~[WIP][Spark-16609] Add to_date with format function.~~ [WIP][SPARK-16609] Add to_date/to_timestamp with format function Dec 4, 2016

bllchmbrs changed the title ~~[WIP][SPARK-16609] Add to_date/to_timestamp with format function~~ [WIP][SPARK-16609] Add to_date/to_timestamp with format functions Dec 4, 2016

bllchmbrs commented Dec 4, 2016

View reviewed changes

added timestamp casting

9a10a34

bllchmbrs added 3 commits December 4, 2016 13:11

formatting, remove extra case

ecde7b7

Merge branch 'master' of https://github.com/apache/spark into SPARK-1…

24a3437

…6609

reset expression work, now just functions. Revert this if going to ed…

89dccbd

…it expressions

bllchmbrs added 2 commits December 4, 2016 14:40

adding tests

c17c0bd

getting closer to all tests

5bf5002

bllchmbrs commented Dec 4, 2016

View reviewed changes

cloud-fan reviewed Jan 26, 2017

View reviewed changes

anabranch added 2 commits February 2, 2017 22:34

R fixes

38c65ca

finally passing tests!!!!

a2d0221

cloud-fan reviewed Feb 3, 2017

View reviewed changes

fixed test

479243f

actually this time

e00b446

ugh, linting

da16c66

maybe someday this will pass

c3063a2

cloud-fan reviewed Feb 6, 2017

View reviewed changes

asfgit closed this in 7a7ce27 Feb 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-16609] Add to_date/to_timestamp with format functions #16138

[SPARK-16609] Add to_date/to_timestamp with format functions #16138

bllchmbrs commented Dec 4, 2016 •

edited

bllchmbrs Dec 4, 2016

bllchmbrs Dec 4, 2016

bllchmbrs Dec 4, 2016

SparkQA commented Dec 4, 2016

rxin commented Dec 4, 2016

bllchmbrs Dec 4, 2016

SparkQA commented Dec 4, 2016

SparkQA commented Dec 4, 2016

bllchmbrs Dec 4, 2016

bllchmbrs Dec 5, 2016

bllchmbrs Dec 5, 2016

SparkQA commented Dec 4, 2016

SparkQA commented Dec 5, 2016

cloud-fan Jan 26, 2017

bllchmbrs Feb 3, 2017

SparkQA commented Feb 3, 2017

cloud-fan Feb 3, 2017

bllchmbrs Feb 3, 2017

hvanhovell commented Feb 3, 2017

SparkQA commented Feb 3, 2017

SparkQA commented Feb 4, 2017

SparkQA commented Feb 4, 2017

SparkQA commented Feb 5, 2017

felixcheung commented Feb 5, 2017 •

edited

bllchmbrs commented Feb 6, 2017

cloud-fan Feb 6, 2017

SparkQA commented Feb 6, 2017

bllchmbrs commented Feb 7, 2017

bllchmbrs commented Feb 7, 2017

cloud-fan commented Feb 7, 2017

hvanhovell commented Feb 7, 2017

YannByron commented Aug 25, 2017

bllchmbrs commented Aug 30, 2017 via email

[SPARK-16609] Add to_date/to_timestamp with format functions #16138

[SPARK-16609] Add to_date/to_timestamp with format functions #16138

Conversation

bllchmbrs commented Dec 4, 2016 • edited

What changes were proposed in this pull request?

Date Function

Timestamp Function

Tasks

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 4, 2016

rxin commented Dec 4, 2016

Choose a reason for hiding this comment

SparkQA commented Dec 4, 2016

SparkQA commented Dec 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 4, 2016

SparkQA commented Dec 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hvanhovell commented Feb 3, 2017

SparkQA commented Feb 3, 2017

SparkQA commented Feb 4, 2017

SparkQA commented Feb 4, 2017

SparkQA commented Feb 5, 2017

felixcheung commented Feb 5, 2017 • edited

bllchmbrs commented Feb 6, 2017

Choose a reason for hiding this comment

SparkQA commented Feb 6, 2017

bllchmbrs commented Feb 7, 2017

bllchmbrs commented Feb 7, 2017

cloud-fan commented Feb 7, 2017

hvanhovell commented Feb 7, 2017

YannByron commented Aug 25, 2017

bllchmbrs commented Aug 30, 2017 via email

bllchmbrs commented Dec 4, 2016 •

edited

felixcheung commented Feb 5, 2017 •

edited