-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-53956][PYTHON] Support TIME in the try_make_timestamp function in PySpark #52666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhengruifeng @HyukjinKwon Please review.
Also, here is the sibling PR for make_timestamp
in PySpark: #52648.
cc @Yicong-Huang can you help review? thanks |
Sure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+----------------------------------------------------+ | ||
|2014-12-28 06:30:45.887 | | ||
+----------------------------------------------------+ | ||
>>> spark.conf.unset("spark.sql.session.timeZone") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems that it was a mistake to introduce this line from the beginning (as time zone is not being used in the example), but want to double confirm this deletion is intentional.
|try_make_timestamp(date, time, tz)| | ||
+----------------------------------+ | ||
|2014-12-27 21:30:45.887 | | ||
+----------------------------------+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to add back >>> spark.conf.unset("spark.sql.session.timeZone")
?
mins: Optional["ColumnOrName"] = None, | ||
secs: Optional["ColumnOrName"] = None, | ||
timezone: Optional["ColumnOrName"] = None, | ||
*, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we had a follow up PR #52461 to remove the *
to make date
and time
also positional. shall we make it consistent?
What changes were proposed in this pull request?
Implement the support for TIME type in
try_make_timestamp
function in PySpark API.Why are the changes needed?
Expand API support for the
TryMakeTimestamp
expression.Does this PR introduce any user-facing change?
Yes, the new function is now available in PySpark API.
How was this patch tested?
Added appropriate Python functions tests and examples.
Was this patch authored or co-authored using generative AI tooling?
No.