HyukjinKwon commented on code in PR #59298:
URL: https://github.com/apache/spark/pull/59298#discussion_r4238701577
##########
python/pyspark/sql/types.py:
##########
@@ -492,10 +518,11 @@ def needConversion(self) -> bool:
def toInternal(self, dt: datetime.datetime) -> int:
if dt is not None:
- seconds = (
- calendar.timegm(dt.utctimetuple()) if dt.tzinfo else
time.mktime(dt.timetuple())
- )
- return int(seconds) * 1000000 + dt.microsecond
+ if dt.tzinfo:
+ seconds = calendar.timegm(dt.utctimetuple())
+ else:
+ seconds = _naive_dt_to_epoch_seconds(dt)
Review Comment:
**Nit (P3):** `toInternal` now resolves a naive repeated-hour datetime by
its `fold`. The classic Py4J input converter `DatetimeConverter.convert`
(types.py:4127-4139) still computes `time.mktime(obj.timetuple())`, and
`timetuple()` drops `fold`, so both occurrences map to one instant there.
Classic `F.lit()` and Column comparisons with a Python datetime go through
that converter. Spark Connect `F.lit()` (connect/expressions.py:299-301) and
classic `createDataFrame` go through `toInternal`.
Under America/Los_Angeles, take `dt = datetime(2021, 11, 7, 1, 30, fold=1)`.
Classic `lit(dt)` becomes 08:30Z, but Connect `lit(dt)` and
`createDataFrame([(dt,)], "t timestamp")` give 09:30Z. So in classic mode
`df.filter(col("t") == dt)` over that DataFrame misses the row, and the same
filter returns different rows in classic and Connect. Before this PR, all three
paths used the same `mktime` expression and agreed.
Using `_naive_dt_to_epoch_seconds(obj)` for the naive branch of
`DatetimeConverter.convert` would keep every path consistent.
**Verification:**
- **Regression:** In classic mode under America/Los_Angeles, filtering a
createDataFrame column of a naive 2021-11-07 01:30 fold=1 datetime with an
equality against lit() of the same value matches the row, and so does fold=0.
- **Compatibility:** Aware and non-ambiguous naive datetimes passed through
classic lit() resolve to the same instants as before the change.
##########
python/pyspark/sql/conversion.py:
##########
@@ -1429,7 +1430,20 @@ def convert_timestamp(value: Any) -> Any:
# lossless path), so drop any sub-microsecond digits to
match classic collect().
if hasattr(value, "to_pydatetime"):
value = value.to_pydatetime(warn=False)
- return value.astimezone().replace(tzinfo=None)
+ # SPARK-60081: integer POSIX seconds so fromtimestamp sets
fold on the
+ # local wall time, then re-attach microseconds. A float
timestamp would
+ # drop microseconds past ~2255 and can overflow at
9999-12-31 under UTC.
Review Comment:
**Nit (P3):** Float seconds start losing microseconds at 2^33 epoch seconds,
which is 2242-03-16, not ~2255. ~2255 is 2^53 microseconds, the limit for an
integer microsecond count in a double. On CPython 3.12, float round trips are
exact just below 2^33 and drift for about half of the samples just above it.
The test comment in `test_arrow_convert_timestamp_range_extremes` already says
~2^33. The ~2255 figure came from my earlier comment, sorry about that.
```suggestion
# drop microseconds past ~2242 and can overflow at
9999-12-31 under UTC.
```
**Verification:**
- **Inspection:** The converter comment names a boundary consistent with
2^33 epoch seconds and no longer says ~2255.
--
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]
