Skip to content

[SPARK-26978][SQL][FOLLOWUP] Initialize date-time constants by foldable expressions - #25116

Closed
MaxGekk wants to merge 1 commit into
apache:masterfrom
MaxGekk:datetime-consts-init
Closed

[SPARK-26978][SQL][FOLLOWUP] Initialize date-time constants by foldable expressions#25116
MaxGekk wants to merge 1 commit into
apache:masterfrom
MaxGekk:datetime-consts-init

Conversation

@MaxGekk

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Reverted initialization of date-time constants in DateTimeUtils introduced by #23878. As a comment in Delta repo states, the compiler can do additional optimizations if values can be calculated at compile time: https://github.com/delta-io/delta/blob/master/src/main/scala/org/apache/spark/sql/delta/util/DateTimeUtils.scala#L63-L75

How was this patch tested?

This was tested by existing test suites.

@MaxGekk

Copy link
Copy Markdown
MemberAuthor

@srowen@hvanhovell Please, review the PR.

@hvanhovellhvanhovell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - pending jenkins

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM, too.
This is the same logic from delta/DateTimeUtils.scala.

@SparkQA

Copy link
Copy Markdown

Test build #107537 has finished for PR 25116 at commit b3bd3a3.

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

@hvanhovell

Copy link
Copy Markdown
Contributor

Merging to master.

vinodkc pushed a commit to vinodkc/spark that referenced this pull request Jul 18, 2019
…le expressions
## What changes were proposed in this pull request?
Reverted initialization of date-time constants in `DateTimeUtils` introduced by apache#23878. As a comment in [Delta repo](https://github.com/delta-io/delta) states, the compiler can do additional optimizations if values can be calculated at compile time: https://github.com/delta-io/delta/blob/master/src/main/scala/org/apache/spark/sql/delta/util/DateTimeUtils.scala#L63-L75
## How was this patch tested?
This was tested by existing test suites.
Closesapache#25116 from MaxGekk/datetime-consts-init.
Authored-by: Maxim Gekk <maxim.gekk@databricks.com>
Signed-off-by: herman <herman@databricks.com>
@MaxGekk
MaxGekk deleted the datetime-consts-init branch September 18, 2019 15:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@MaxGekk@SparkQA@hvanhovell@srowen@dongjoon-hyun