Skip to content

Conversation

uros-db
Copy link
Contributor

@uros-db uros-db commented Oct 20, 2025

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.

@uros-db uros-db changed the title [WIP][SPARK-53956][SQL][PYTHON Support TIME in the try_make_timestamp function in PySpark [WIP][SPARK-53956][PYTHON] Support TIME in the try_make_timestamp function in PySpark Oct 20, 2025
@uros-db uros-db changed the title [WIP][SPARK-53956][PYTHON] Support TIME in the try_make_timestamp function in PySpark [SPARK-53956][PYTHON] Support TIME in the try_make_timestamp function in PySpark Oct 20, 2025
Copy link
Contributor Author

@uros-db uros-db left a 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.

@zhengruifeng
Copy link
Contributor

cc @Yicong-Huang can you help review? thanks

@Yicong-Huang
Copy link
Contributor

cc @Yicong-Huang can you help review? thanks

Sure

Copy link
Contributor

@Yicong-Huang Yicong-Huang left a comment

Choose a reason for hiding this comment

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

LGTM! The changes are mostly consistent with #51831. But please consider align with #52461 as well.

+----------------------------------------------------+
|2014-12-28 06:30:45.887 |
+----------------------------------------------------+
>>> spark.conf.unset("spark.sql.session.timeZone")
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this change?

Copy link
Contributor

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 |
+----------------------------------+
Copy link
Contributor

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,
*,
Copy link
Contributor

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?

Sign up for free to 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.

3 participants