-
Notifications
You must be signed in to change notification settings - Fork 271
Stop parsing special dates for Spark 3.2+ #3439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
33637dd
000ce39
d87804e
ec0f802
a8fc234
54a0059
926a824
111620d
fec39d5
b2f35d3
28585b2
790eb44
76eaa44
50e85ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -241,11 +241,8 @@ class ParseDateTimeSuite extends SparkQueryCompareTestSuite with BeforeAndAfterE | |
| .repartition(2) | ||
| .withColumn("c1", unix_timestamp(col("c0"), "yyyy-MM-dd HH:mm:ss")) | ||
| } | ||
| val startTimeSeconds = System.currentTimeMillis()/1000L | ||
| val cpuNowSeconds = withCpuSparkSession(now).collect().head.toSeq(1).asInstanceOf[Long] | ||
| val gpuNowSeconds = withGpuSparkSession(now).collect().head.toSeq(1).asInstanceOf[Long] | ||
| assert(cpuNowSeconds >= startTimeSeconds) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reason for removing these assertions? Are they no longer valid?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, under spark 3.2+, the result will be zero instead of current time, since
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then have the test check the version. and have it check that now is replaced with a 0 instead. Or just have the test skip entirely if it is 3.2+ |
||
| assert(gpuNowSeconds >= startTimeSeconds) | ||
| // CPU ran first so cannot have a greater value than the GPU run (but could be the same second) | ||
| assert(cpuNowSeconds <= gpuNowSeconds) | ||
| } | ||
|
|
@@ -534,4 +531,3 @@ class ParseDateTimeSuite extends SparkQueryCompareTestSuite with BeforeAndAfterE | |
| ) | ||
|
|
||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems expensive to replace the special dates with null with Spark 3.2 rather than just skip handling them at all. They will already be ignored by
is_timestamp.I will pull this PR locally today and experiment with it and come back with more detailed suggestions.