Skip to content

Conversation

@hvanhovell
Copy link
Contributor

@hvanhovell hvanhovell commented Jun 9, 2016

What changes were proposed in this pull request?

The current implementations of UnixTime and FromUnixTime do not cache their parser/formatter as much as they could. This PR resolved this issue.

This PR is a take over from #13522 and further optimizes the re-use of the parser/formatter. It also fixes the improves handling (catching the actual exception instead of Throwable). All credits for this work should go to @rajeshbalamohan.

This PR closes #13522

How was this patch tested?

Current tests.

@hvanhovell
Copy link
Contributor Author

cc @cloud-fan

val formatter = ctx.freshName("formatter")
if (fString == null) {
if (formatter == null) {
ev.copy(code = s"""
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be: ev.copy(code = "", isNull = "false", value = ctx.defaultValue(dataType))

@cloud-fan
Copy link
Contributor

LGTM except a minor comment

@JoshRosen
Copy link
Contributor

Tip: if you put "closes #13522" in the PR description then the subsumed PR will be automatically closed once this PR is merged.

@hvanhovell hvanhovell changed the title [SPARK-14321][SQL] Reduce date format cost and string-to-date cost in date functions [SPARK-14321][SQL] Reduce date format cost and string-to-date cost in date functions - closes #13522 Jun 9, 2016
@cloud-fan
Copy link
Contributor

in the PR description

@hvanhovell not PR title...

@cloud-fan
Copy link
Contributor

LGTM, pending jenkins

@hvanhovell hvanhovell changed the title [SPARK-14321][SQL] Reduce date format cost and string-to-date cost in date functions - closes #13522 [SPARK-14321][SQL] Reduce date format cost and string-to-date cost in date functions Jun 9, 2016
@SparkQA
Copy link

SparkQA commented Jun 9, 2016

Test build #60245 has finished for PR 13581 at commit a086128.

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

@SparkQA
Copy link

SparkQA commented Jun 9, 2016

Test build #60247 has finished for PR 13581 at commit 0ca9f99.

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

@rxin
Copy link
Contributor

rxin commented Jun 9, 2016

Merging in master/2.0.

asfgit pushed a commit that referenced this pull request Jun 9, 2016
… date functions

## What changes were proposed in this pull request?
The current implementations of `UnixTime` and `FromUnixTime` do not cache their parser/formatter as much as they could. This PR resolved this issue.

This PR is a take over from #13522 and further optimizes the re-use of the parser/formatter. It also fixes the improves handling (catching the actual exception instead of `Throwable`). All credits for this work should go to rajeshbalamohan.

This PR closes #13522

## How was this patch tested?
Current tests.

Author: Herman van Hovell <hvanhovell@databricks.com>
Author: Rajesh Balamohan <rbalamohan@apache.org>

Closes #13581 from hvanhovell/SPARK-14321.

(cherry picked from commit b076853)
Signed-off-by: Reynold Xin <rxin@databricks.com>
@asfgit asfgit closed this in b076853 Jun 9, 2016
zjffdu pushed a commit to zjffdu/spark that referenced this pull request Jun 10, 2016
… date functions

## What changes were proposed in this pull request?
The current implementations of `UnixTime` and `FromUnixTime` do not cache their parser/formatter as much as they could. This PR resolved this issue.

This PR is a take over from apache#13522 and further optimizes the re-use of the parser/formatter. It also fixes the improves handling (catching the actual exception instead of `Throwable`). All credits for this work should go to rajeshbalamohan.

This PR closes apache#13522

## How was this patch tested?
Current tests.

Author: Herman van Hovell <hvanhovell@databricks.com>
Author: Rajesh Balamohan <rbalamohan@apache.org>

Closes apache#13581 from hvanhovell/SPARK-14321.
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.

6 participants