Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

in #24195 , we deprecate from/to_utc_timestamp.

This PR removes unnecessary use of to_utc_timestamp in the test.

How was this patch tested?

test only PR

@cloud-fan
Copy link
Contributor Author

cc @MaxGekk @srowen

val inputData = MemoryStream[Long]
val aggregated =
inputData.toDF()
.select(($"value" * DateTimeUtils.SECONDS_PER_DAY).cast("timestamp").as("value"))
Copy link
Member

Choose a reason for hiding this comment

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

This is basically the only change right? can you remove tz too?
Seems OK, especially if pulling timezones out of the equation here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is one more: I replaced current_date with current_timestamp().cast("date").

current_date returns date in UTC. We can't compare it with the value of casting timestamp column to date. There is a timezone shift when casting between date and timestamp.

@SparkQA
Copy link

SparkQA commented Apr 8, 2019

Test build #104391 has finished for PR 24319 at commit f9bc375.

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

@SparkQA
Copy link

SparkQA commented Apr 8, 2019

Test build #104389 has finished for PR 24319 at commit 3ae2f44.

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

Copy link
Contributor

@attilapiros attilapiros left a comment

Choose a reason for hiding this comment

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

LGTM

Tip for the next reviewer: after switching on "Hide whitespace changes" the diff is quite small and easier to review.

@cloud-fan
Copy link
Contributor Author

thanks, merging to master!

@cloud-fan cloud-fan closed this in 051336d Apr 9, 2019
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