Skip to content

Conversation

@younggyuchun
Copy link

@younggyuchun younggyuchun commented Feb 21, 2020

What changes were proposed in this pull request?

This PR aims to replace functions in DateTimeUtils that convert dates to/from "timestamp" in millisecond precision.

  1. Replace millisToDays by microsToDays in DateTimeUtils.
  2. Replace daysToMillis by daysToMicros in DateTimeUtils.

Why are the changes needed?

Spark should reduce overhead costs to convert dates to a timestamp in milliseconds as TimestamType values in the Catalyst are stored as microseconds. Recently we switched to Java 8 time API so we do not need to convert to milliseconds.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass the Jenkins with the existing tests.

@younggyuchun younggyuchun marked this pull request as ready for review February 21, 2020 02:10
@younggyuchun
Copy link
Author

younggyuchun commented Feb 21, 2020

I can't add MaxGekk as a reviewer. cc @MaxGekk

@SparkQA
Copy link

SparkQA commented Feb 21, 2020

Test build #118741 has finished for PR 27661 at commit 0fd79de.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@younggyuchun younggyuchun removed the request for review from dongjoon-hyun February 21, 2020 02:47
@SparkQA
Copy link

SparkQA commented Feb 21, 2020

Test build #118744 has finished for PR 27661 at commit 130baec.

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

@MaxGekk
Copy link
Member

MaxGekk commented Feb 21, 2020

This PR is a duplicate work of my PR #27618 opened 3 days ago. I would like to kindly ask you to close it. /cc @cloud-fan @dongjoon-hyun @srowen @HyukjinKwon

@HyukjinKwon
Copy link
Member

Let's close and review #27618

@younggyuchun younggyuchun deleted the SPARK-30869 branch February 21, 2020 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants