[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows #36566

dingsl-giser · 2022-05-16T09:38:09Z

What changes were proposed in this pull request?

Fix problems with pyspark in Windows:

Fixed datetime conversion to timestamp before 1970;
Fixed datetime conversion when timestamp is negative;
Adding a test script.

Why are the changes needed?

Pyspark has problems serializing pre-1970 times in Windows.

An exception occurs when executing the following code under Windows:

rdd = sc.parallelize([('a', datetime(1957, 1, 9, 0, 0)),
                      ('b', datetime(2014, 1, 27, 0, 0))])
df = spark.createDataFrame(rdd, ["id", "date"])

df.show()
df.printSchema()

print(df.collect())

  File "...\spark\python\lib\pyspark.zip\pyspark\sql\types.py", line 195, in toInternal
    else time.mktime(dt.timetuple()))
OverflowError: mktime argument out of range

at org.apache.spark.api.python.BasePythonRunner$ReaderIterator.handlePythonException(PythonRunner.scala:503)
	at org.apache.spark.api.python.PythonRunner$$anon$3.read(PythonRunner.scala:638)
	at org.apache.spark.api.python.PythonRunner$$anon$3.read(PythonRunner.scala:621)
	at org.apache.spark.api.python.BasePythonRunner$ReaderIterator.hasNext(PythonRunner.scala:456)
	at org.apache.spark.InterruptibleIterator.hasNext(InterruptibleIterator.scala:37)
	at scala.collection.Iterator$$anon$11.hasNext(Iterator.scala:489)
	at scala.collection.Iterator$$anon$10.hasNext(Iterator.scala:458)
	at scala.collection.Iterator$$anon$10.hasNext(Iterator.scala:458)
	at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source)
	at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43)
	at org.apache.spark.sql.execution.WholeStageCodegenExec$$anon$1.hasNext(WholeStageCodegenExec.scala:729)
	at org.apache.spark.sql.execution.SparkPlan.$anonfun$getByteArrayRdd$1(SparkPlan.scala:340)
	at org.apache.spark.rdd.RDD.$anonfun$mapPartitionsInternal$2(RDD.scala:872)
	at org.apache.spark.rdd.RDD.$anonfun$mapPartitionsInternal$2$adapted(RDD.scala:872)
	at org.apache.spark.rdd.MapPartitionsRDD.compute(MapPartitionsRDD.scala:52)
	at org.apache.spark.rdd.RDD.computeOrReadCheckpoint(RDD.scala:349)
	at org.apache.spark.rdd.RDD.iterator(RDD.scala:313)
	at org.apache.spark.scheduler.ResultTask.runTask(ResultTask.scala:90)
	at org.apache.spark.scheduler.Task.run(Task.scala:127)
	at org.apache.spark.executor.Executor$TaskRunner.$anonfun$run$3(Executor.scala:446)
	at org.apache.spark.util.Utils$.tryWithSafeFinally(Utils.scala:1377)
	at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:449)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	... 1 more

and

File ...\spark\python\lib\pyspark.zip\pyspark\sql\types.py, in fromInternal:
Line 207:   return datetime.datetime.fromtimestamp(ts // 1000000).replace(microsecond=ts % 1000000)

OSError: [Errno 22] Invalid argument

After updating the code, the above code was run successfully！

+---+-------------------+
| id|               date|
+---+-------------------+
|  a|1957-01-08 16:00:00|
|  b|2014-01-26 16:00:00|
+---+-------------------+

root
 |-- id: string (nullable = true)
 |-- date: timestamp (nullable = true)

[Row(id='a', date=datetime.datetime(1957, 1, 9, 0, 0)), Row(id='b', date=datetime.datetime(2014, 1, 27, 0, 0))]

Does this PR introduce any user-facing change?

No

How was this patch tested?

New and existing test suites

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

dingsl-giser · 2022-05-16T10:49:36Z

@HyukjinKwon I closed the RP in the 3.0 branch(#36537) and raised a new RP in the master branch.

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

HyukjinKwon · 2022-05-17T03:16:17Z

python/pyspark/sql/types.py

-            seconds = (
-                calendar.timegm(dt.utctimetuple()) if dt.tzinfo else time.mktime(dt.timetuple())
-            )
+            if platform.system().lower() == 'windows':


Should we also check if the value is negative?

Don't need, this method is still available when the negative.

How about performance? This is hot path that's executed every value, and the performance here is pretty critical.

Are there professional performance testers?

HyukjinKwon · 2022-05-17T03:16:59Z

@AnywalkerGiser BTW, it would be easier to follow and review if you have any link about describing the OS difference in that Python library.

dingsl-giser · 2022-05-17T03:33:12Z

@HyukjinKwon Do you mean the platform library, added to the code comments?

HyukjinKwon · 2022-05-17T03:40:34Z

I am surprised that the same Python library returns a different value depending on OS. I know some libraries are dependent on C library implementation but didn't expect such drastic difference. Code comment or documentation anything is fine. I would like to see if this is an official difference in Python.

dingsl-giser · 2022-05-17T03:52:23Z

@HyukjinKwon Python3 datetime interface documentation
This is explained in this document.

HyukjinKwon · 2022-05-17T03:55:50Z

So is C localtime function has a different behaviour in OS? that returns negative values?

dingsl-giser · 2022-05-17T04:15:34Z

The localtime function does not differ in OS, the fromtimestamp function does, and datetime many functions have problems resolving dates before 1970 in windows.

dingsl-giser · 2022-05-17T04:16:31Z

@HyukjinKwon Can this solution be merged into master?

AmplabJenkins · 2022-05-17T04:21:37Z

Can one of the admins verify this patch?

HyukjinKwon · 2022-05-17T05:11:07Z

I would like to understand the problem first, and see if this behaviour difference is intentional or official. This is core code path so would have to expect more reviews and time.

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

dingsl-giser · 2022-05-17T05:18:45Z

Understood, looking forward to your reply later.

HyukjinKwon · 2022-05-17T05:19:26Z

@AnywalkerGiser mind pointing out any documentation that states this negative value?

dingsl-giser · 2022-05-17T05:40:24Z

Here are some blogs on related issues:
Python | mktime overflow error
Python fromtimestamp OSError

dingsl-giser · 2022-05-17T05:43:05Z

I think the Spark project can look for a solution if Python doesn't fix this bug.

HyukjinKwon · 2022-05-17T05:45:18Z

Does that happen in all Windows with all Python versions?

HyukjinKwon · 2022-05-17T05:45:34Z

is this a bug in Python?

dingsl-giser · 2022-05-17T05:58:52Z

I thought it was a bug in python, but the documentation said it would report an error if it was out of time range. I tested python 3.6, 3.7, and 3.8 on windows.

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

srowen · 2022-05-18T13:36:50Z

python/pyspark/sql/types.py

-            seconds = (
-                calendar.timegm(dt.utctimetuple()) if dt.tzinfo else time.mktime(dt.timetuple())
-            )
+            if platform.system().lower() == "windows":


I'm just super surprised this is Windows specific?

Yes, and I want you to check it out for yourself.

github-actions · 2022-10-10T00:29:15Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

dingsl-giser · 2023-12-08T09:50:51Z

This problem hasn't been solved yet?

dingsl-giser · 2023-12-08T09:53:42Z

@HyukjinKwon This problem still exists in the new version. Can it be merged?

dingsl-giser added 2 commits May 16, 2022 17:33

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1…

2c03072

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

6dcf936

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

github-actions bot added CORE PYTHON SQL labels May 16, 2022

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

5106f9e

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

dingsl-giser added 3 commits May 16, 2022 22:35

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

3c5e52a

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1…

c96b839

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1…

06c3a32

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

HyukjinKwon reviewed May 17, 2022

View reviewed changes

Merge branch 'apache:master' into master

23d0a0d

dingsl-giser added 3 commits May 17, 2022 13:12

Merge branch 'apache:master' into master

c9305be

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1…

866ebf7

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

39e9510

[SPARK-39176][PYSPARK] Pre - 1970 time serialization test

dingsl-giser added 2 commits May 17, 2022 14:19

Merge branch 'apache:master' into master

edbd03a

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1…

d47a175

…970 datetime in windows [SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows

dingsl-giser requested a review from HyukjinKwon May 18, 2022 03:29

srowen reviewed May 18, 2022

View reviewed changes

dingsl-giser requested a review from srowen May 27, 2022 01:58

dingsl-giser mentioned this pull request Jul 1, 2022

[SPARK-39347] [SS] Generate wrong time window when (timestamp-startTime) % slideDuration… #36737

Closed

Merge branch 'apache:master' into master

d2b5201

alecuba16 mentioned this pull request Aug 24, 2022

Fixed timestamp conversion on Windows #37647

Closed

github-actions bot added the Stale label Oct 10, 2022

github-actions bot closed this Oct 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows #36566

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows #36566

dingsl-giser commented May 16, 2022

dingsl-giser commented May 16, 2022

HyukjinKwon May 17, 2022

dingsl-giser May 17, 2022

HyukjinKwon May 17, 2022

dingsl-giser Jul 1, 2022

HyukjinKwon commented May 17, 2022 •

edited

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

dingsl-giser commented May 17, 2022

AmplabJenkins commented May 17, 2022

HyukjinKwon commented May 17, 2022 •

edited

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

srowen May 18, 2022

dingsl-giser May 18, 2022

github-actions bot commented Oct 10, 2022

dingsl-giser commented Dec 8, 2023

dingsl-giser commented Dec 8, 2023

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows #36566

[SPARK-39176][PYSPARK] Fixed a problem with pyspark serializing pre-1970 datetime in windows #36566

Conversation

dingsl-giser commented May 16, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

dingsl-giser commented May 16, 2022

HyukjinKwon May 17, 2022

Choose a reason for hiding this comment

dingsl-giser May 17, 2022

Choose a reason for hiding this comment

HyukjinKwon May 17, 2022

Choose a reason for hiding this comment

dingsl-giser Jul 1, 2022

Choose a reason for hiding this comment

HyukjinKwon commented May 17, 2022 • edited

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

dingsl-giser commented May 17, 2022

AmplabJenkins commented May 17, 2022

HyukjinKwon commented May 17, 2022 • edited

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

dingsl-giser commented May 17, 2022

HyukjinKwon commented May 17, 2022

HyukjinKwon commented May 17, 2022

dingsl-giser commented May 17, 2022

srowen May 18, 2022

Choose a reason for hiding this comment

dingsl-giser May 18, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2022

dingsl-giser commented Dec 8, 2023

dingsl-giser commented Dec 8, 2023

HyukjinKwon commented May 17, 2022 •

edited

HyukjinKwon commented May 17, 2022 •

edited