feat: Implement Spark-compatible CAST from string to timestamp types

[vaibhawvipul]

Which issue does this PR close?

Closes #328 .

Rationale for this change

Improve compatibility with Apache Spark

What changes are included in this PR?

Add custom implementation of CAST from string to timestamp rather than delegate to DataFusion

How are these changes tested?

• added unit test cases

[andygrove]

@vaibhawvipul You can run make format to fix the linting issues that are causing some CI tests to fail

[andygrove]

The regex approach is a good way to quickly get support for all of the format variations and fix the correctness issue but could also be quite expensive. It would be good to add some criterion benchmarks so that we can understand what performance looks like (does not have to be part of this PR).

[vaibhawvipul]

Update -

Working on this bug, there is 8 hours difference in comet vs spark cast. If anyone has any pointer, please let me know.

[andygrove]

Update -

Working on this bug, there is 8 hours difference in comet vs spark cast. If anyone has any pointer, please let me know.

What happens if you explicitly set the timezone to UTC in the Spark configs?

We may want to fallback to Spark for other timezones for now.

[vaibhawvipul]

Thanks @andygrove , setting timestamp as UTC worked!

Any idea about this error?

test("cast string to timestamp") {
    withSQLConf(
      (SQLConf.SESSION_LOCAL_TIMEZONE.key -> "UTC"),
      (CometConf.COMET_CAST_STRING_TO_TIMESTAMP.key -> "true")) {
      val values = Seq("2020-01-01T12:34:56.123456", "T2").toDF("a")
      castTest(values, DataTypes.TimestampType)
    }
  }

[vaibhawvipul]

seems like, test is failing in ANSI mode, debugging that.

[vaibhawvipul]

@andygrove

I have enabled a "string to timestamp" cast test case that passes. We are supporting all the timestamps mentioned in the issue.

Input	Spark	Comet
2020	2020-01-01 00:00:00.0	2020-01-01 00:00:00.0
2020-01	2020-01-01 00:00:00.0	2020-01-01 00:00:00.0
2020-01-01	2020-01-01 00:00:00.0	2020-01-01 00:00:00.0
2020-01-01T12	2020-01-01 12:00:00.0	2020-01-01 12:00:00.0
2020-01-01T12:34	2020-01-01 12:34:00.0	2020-01-01 12:34:00.0
2020-01-01T12:34:56	2020-01-01 12:34:56.0	2020-01-01 12:34:56.0
2020-01-01T12:34:56.123456	2020-01-01 12:34:56.123456	2020-01-01 12:34:56.123456
T2	2024-04-25 02:00:00.0	2024-04-25 02:00:00.0

Just a note -

• Spark supports some additional inputs like - "213" or "85678". i.e 3 digit and 5 digit years too! This is not being supported right now by Comet in this PR.

[andygrove]

seems like, test is failing in ANSI mode, debugging that.

There is an improved version of the ANSI testing in #351

[andygrove]

Thanks @vaibhawvipul this is looking great. I will review today.

[andygrove]

• Spark supports some additional inputs like - "213" or "85678". i.e 3 digit and 5 digit years too! This is not being supported right now by Comet in this PR.

I would be fine with handling this in a follow on PR

[andygrove]

This is looking great @vaibhawvipul. I think this is close to being ready to merge and then have some follow on issues for remaining items.

I think the one thing I would like to see before merging is that we can run the fuzz test without causing any panics, so just replacing unwraps with error handling.

[vaibhawvipul]

This is looking great @vaibhawvipul. I think this is close to being ready to merge and then have some follow on issues for remaining items.

I think the one thing I would like to see before merging is that we can run the fuzz test without causing any panics, so just replacing unwraps with error handling.

@andygrove all unwraps are removed, I am handling errors in all of my functions.

[andygrove]

Thank you @vaibhawvipul. I think this is at a good point to merge and then we can file issues for follow up items, such as:

• Add support for 3 and 5 digit years
• Testing with try_cast and ANSI
• Passing all fuzz tests
• Updating compatibility guide

[parthchandra]

As a follow up, I would also suggest adding/verifying support for any timezone. (See this test for instance:

datafusion-comet/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala

Line 262 in 064cb47

withSQLConf(SESSION_LOCAL_TIMEZONE.key -> "Asia/Kathmandu") {

)

Also see - https://docs.rs/arrow-cast/50.0.0/arrow_cast/parse/fn.string_to_datetime.html

[andygrove]

I filed #376 for the follow on work