Skip to content

Conversation

@tmnd1991
Copy link
Contributor

@tmnd1991 tmnd1991 commented Jul 2, 2018

What changes were proposed in this pull request?

Add an overloaded version to from_utc_timestamp and to_utc_timestamp having second argument as a Column instead of String.

How was this patch tested?

Unit testing, especially adding two tests to org.apache.spark.sql.DateFunctionsSuite.scala

@srowen
Copy link
Member

srowen commented Jul 2, 2018

See https://spark-prs.appspot.com/ ; you'll want to fix up the title of this PR.

@tmnd1991 tmnd1991 changed the title [SPARK 24673] [SPARK-24673][SQL] scala sql function from_utc_timestamp second argument could be Column instead of String Jul 2, 2018
* zone, and renders that time as a timestamp in UTC. For example, 'GMT+1' would yield
* '2017-07-14 01:40:00.0'.
* @group datetime_funcs
* @since 1.5.0
Copy link
Member

@maropu maropu Jul 3, 2018

Choose a reason for hiding this comment

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

ditto

* that time as a timestamp in the given time zone. For example, 'GMT+1' would yield
* '2017-07-14 03:40:00.0'.
* @group datetime_funcs
* @since 1.5.0
Copy link
Member

Choose a reason for hiding this comment

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

@since 2.4.0

@maropu
Copy link
Member

maropu commented Jul 3, 2018

cc: @ueshin @gatorsmile

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

The overload seems reasonable to me.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented Jul 3, 2018

Test build #4204 has finished for PR 21693 at commit d4ebc8f.

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

* @group datetime_funcs
* @since 2.4.0
*/
def from_utc_timestamp(ts: Column, tz: Column): Column = withExpr {
Copy link
Member

@HyukjinKwon HyukjinKwon Jul 5, 2018

Choose a reason for hiding this comment

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

This is being merged since tz column can differ from the constant tz string for each record which makes sense in usecase, and arguably it's a common function.

@HyukjinKwon
Copy link
Member

Merged to master.

@asfgit asfgit closed this in 4be9f0c Jul 5, 2018
@zzcclp
Copy link
Contributor

zzcclp commented Jul 5, 2018

@HyukjinKwon will this pr be merged into branch-2.3/branch-2.2 too?

@maropu
Copy link
Member

maropu commented Jul 5, 2018

Basically, bugs are only backported to the earlier versions.

* @group datetime_funcs
* @since 2.4.0
*/
def from_utc_timestamp(ts: Column, tz: Column): Column = withExpr {
Copy link
Member

Choose a reason for hiding this comment

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

For these changes, we also need to add the corresponding changes in PySpark functions.

@gatorsmile
Copy link
Member

@maropu @HyukjinKwon @zzcclp @tmnd1991 Could any of you submit a follow-up PR for PySpark?

@HyukjinKwon
Copy link
Member

Will go ahead on this weekends. If you guys find some times please go ahead till then.

@maropu
Copy link
Member

maropu commented Jul 6, 2018

ok, I get time now, so I'll take it ;)

@HyukjinKwon
Copy link
Member

Thanks, @maropu.

@maropu
Copy link
Member

maropu commented Jul 6, 2018

@gatorsmile btw, on a personal note, I got a new (& first) baby in my family hours ago ;))

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.

8 participants