Yicong-Huang commented on code in PR #52666:
URL: https://github.com/apache/spark/pull/52666#discussion_r2445653756


##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -24972,7 +25036,6 @@ def try_make_timestamp(
     +----------------------------------------------------+
     |2014-12-28 06:30:45.887                             |
     +----------------------------------------------------+
-    >>> spark.conf.unset("spark.sql.session.timeZone")

Review Comment:
   why is this change?



##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -24972,7 +25036,6 @@ def try_make_timestamp(
     +----------------------------------------------------+
     |2014-12-28 06:30:45.887                             |
     +----------------------------------------------------+
-    >>> spark.conf.unset("spark.sql.session.timeZone")

Review Comment:
   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.



##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -24988,16 +25051,85 @@ def try_make_timestamp(
     |NULL                                                |
     +----------------------------------------------------+
 
+    Example 4: Make timestamp from date, time, and timezone.
+
+    >>> import pyspark.sql.functions as sf
+    >>> from datetime import date, time
+    >>> df = spark.range(1).select(
+    ...     sf.lit(date(2014, 12, 28)).alias("date"),
+    ...     sf.lit(time(6, 30, 45, 887000)).alias("time"),
+    ...     sf.lit("CET").alias("tz")
+    ... )
+    >>> df.select(
+    ...     sf.try_make_timestamp(date=df.date, time=df.time, timezone=df.tz)
+    ... ).show(truncate=False)
+    +----------------------------------+
+    |try_make_timestamp(date, time, tz)|
+    +----------------------------------+
+    |2014-12-27 21:30:45.887           |
+    +----------------------------------+

Review Comment:
   do we need to add back `>>> spark.conf.unset("spark.sql.session.timeZone")`?



##########
python/pyspark/sql/connect/functions/builtin.py:
##########
@@ -3967,23 +3967,99 @@ def make_timestamp(
 make_timestamp.__doc__ = pysparkfuncs.make_timestamp.__doc__
 
 
+@overload
 def try_make_timestamp(
     years: "ColumnOrName",
     months: "ColumnOrName",
     days: "ColumnOrName",
     hours: "ColumnOrName",
     mins: "ColumnOrName",
     secs: "ColumnOrName",
+) -> Column:
+    ...
+
+
+@overload
+def try_make_timestamp(
+    years: "ColumnOrName",
+    months: "ColumnOrName",
+    days: "ColumnOrName",
+    hours: "ColumnOrName",
+    mins: "ColumnOrName",
+    secs: "ColumnOrName",
+    timezone: "ColumnOrName",
+) -> Column:
+    ...
+
+
+@overload
+def try_make_timestamp(*, date: "ColumnOrName", time: "ColumnOrName") -> 
Column:
+    ...
+
+
+@overload
+def try_make_timestamp(
+    *, date: "ColumnOrName", time: "ColumnOrName", timezone: "ColumnOrName"
+) -> Column:
+    ...
+
+
+def try_make_timestamp(
+    years: Optional["ColumnOrName"] = None,
+    months: Optional["ColumnOrName"] = None,
+    days: Optional["ColumnOrName"] = None,
+    hours: Optional["ColumnOrName"] = None,
+    mins: Optional["ColumnOrName"] = None,
+    secs: Optional["ColumnOrName"] = None,
     timezone: Optional["ColumnOrName"] = None,
+    *,

Review Comment:
   we had a follow up PR https://github.com/apache/spark/pull/52461 to remove 
the `*` to make `date` and `time` also positional. shall we make it consistent? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to