[SPARK-33055][PYTHON][SQL] Add Python CalendarIntervalType #29935

zero323 · 2020-10-02T15:28:59Z

What changes were proposed in this pull request?

This PR adds CalendarIntervalType to pyspark.sql.types.

Why are the changes needed?

Right now, queries resulting in interval types, fail with ValueError when _parse_datatype_json_string is invoked:

>>> spark.sql("SELECT current_date() - current_date()")                                                                                                                                  
Traceback (most recent call last):
...
ValueError: Could not parse datatype: interval

This is a serious loss of function.

Does this PR introduce any user-facing change?

If this PR is accepted, example query will yield expected results:

>>> spark.sql("SELECT current_date() - current_date()")
DataFrame[subtractdates(current_date(), current_date()): interval]

Please note that providing a mapping between internal and external type is not a goal here.

How was this patch tested?

New unit test.

SparkQA · 2020-10-02T15:44:17Z

Test build #129354 has finished for PR 29935 at commit d9a6e44.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

SparkQA · 2020-10-02T16:13:45Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33963/

SparkQA · 2020-10-02T16:30:20Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33963/

SparkQA · 2020-10-02T16:52:03Z

Test build #129357 has finished for PR 29935 at commit 80fbf32.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

SparkQA · 2020-10-02T17:24:44Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33967/

SparkQA · 2020-10-02T17:28:24Z

Test build #129359 has finished for PR 29935 at commit 7d6ebd2.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

SparkQA · 2020-10-02T17:42:05Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33967/

SparkQA · 2020-10-02T17:46:10Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/33969/

HyukjinKwon · 2020-10-03T04:06:34Z

python/pyspark/sql/types.py

@@ -186,6 +186,30 @@ def fromInternal(self, ts):
            return datetime.datetime.fromtimestamp(ts // 1000000).replace(microsecond=ts % 1000000)


+class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):


There have been a lot of discussions about exposing interval type in other language APIs but I lost the track. @yaooqinn and @cloud-fan, are we going to make internal as a proper exposed type? Or only support it in some contexts?

Yes, I glanced over a few threads and couldn't really figure out where it is going, hence a limited scope of this PR. However, if the type is supported in multiple contexts, current behavior doesn't seem like an intended one.

Doesn't @zero323's example from the PR description show that Spark already exposes this type?

spark.sql("SELECT current_date() - current_date()")

For the record, btw, Postgres supports an interval type and done so since at least version 7.1, which was released in 2001. (I mention this since Postgres often comes up as a reference for whether Spark SQL should support a feature or not.)

The problem is that it has been half-exposed so far. There have been many discussions up to which context we should support. e.g.) CalendarInterval is marked as Unstable.

BTW, I personally agree with adding this in particular due to @nchammas point #29935 (comment) here

SparkQA · 2020-11-03T11:48:28Z

Test build #130565 has finished for PR 29935 at commit 6ca50c5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

SparkQA · 2020-11-03T12:17:11Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35166/

SparkQA · 2020-11-03T12:41:34Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35166/

SparkQA · 2021-01-07T06:46:00Z

Test build #133778 has finished for PR 29935 at commit 69438b4.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

zero323 · 2021-01-16T10:48:15Z

FYI @MrPowers (in case of your going to work on Python API for make_interval and encounter this).

MrPowers · 2021-01-17T00:35:05Z

@zero323 - Adding CalendarIntervalType to PySpark is a great idea.

Additional context for others: CalendarIntervalType is already in the Scala API and allows for some awesome functionality.

Here's the Spark 3.0.1 behavior with Scala:

import java.sql.Date
import org.apache.spark.sql.functions._
val df = Seq(
  (Date.valueOf("2021-01-23"), Date.valueOf("2021-01-21"))
).toDF("date1", "date2")
df.withColumn("new_datediff", $"date1" - $"date2").show()
//+----------+----------+------------+
//|     date1|     date2|new_datediff|
//+----------+----------+------------+
//|2021-01-23|2021-01-21|      2 days|
//+----------+----------+------------+

df.withColumn("new_datediff", $"date1" - $"date2").printSchema()
//root
// |-- date1: date (nullable = true)
// |-- date2: date (nullable = true)
// |-- new_datediff: interval (nullable = true)

Getting this functionality in PySpark would be a huge win.

Let me know if there is anything I can do to help you move this PR forward @zero323!

nchammas

LGTM. Are we just waiting on a second look from @yaooqinn or @cloud-fan?

nchammas · 2021-01-21T20:38:42Z

python/pyspark/sql/types.py

+    def toInternal(self, di):
+        raise NotImplementedError(
+            "Conversion from external Python types to interval not supported"
+        )


I suppose in the future if we want to support conversion of Python's datetime.timedelta, it would happen here, right?

For full support we might need both Python and JVM component. If I recall correctly timedelta has razrovine mapping to their internal net.razorvine.pickle.objects.TimeDelta.

In the opposite direction we could, if I am not mistaken, start with making CalendarInterval bean compatible, but there is compatibility issue ‒ we'd have to map from Spark's months to Python's days.

nchammas · 2021-01-21T20:38:42Z

python/pyspark/sql/types.py

@@ -186,6 +186,30 @@ def fromInternal(self, ts):
            return datetime.datetime.fromtimestamp(ts // 1000000).replace(microsecond=ts % 1000000)


+class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):


Doesn't @zero323's example from the PR description show that Spark already exposes this type?

spark.sql("SELECT current_date() - current_date()")

For the record, btw, Postgres supports an interval type and done so since at least version 7.1, which was released in 2001. (I mention this since Postgres often comes up as a reference for whether Spark SQL should support a feature or not.)

zero323 · 2021-01-21T21:46:15Z

Let me know if there is anything I can do to help you move this PR forward @zero323!

Thanks @MrPowers! I reckon we mostly need some attention here.

zero323 force-pushed the zero323:SPARK-33055 branch from d9a6e44 to 80fbf32 Oct 2, 2020

zero323 force-pushed the zero323:SPARK-33055 branch from 80fbf32 to 7d6ebd2 Oct 2, 2020

HyukjinKwon reviewed Oct 3, 2020

View changes

zero323 marked this pull request as ready for review Oct 5, 2020

zero323 force-pushed the zero323:SPARK-33055 branch from 7d6ebd2 to 6ca50c5 Nov 3, 2020

Add Python CalendarIntervalType

Verified

This commit was signed with a verified signature.

zero323 Maciej

GPG key ID: A30CEF0C31A501EC Learn about signing commits

Loading status checks…

69438b4

zero323 force-pushed the zero323:SPARK-33055 branch from 6ca50c5 to 69438b4 Dec 1, 2020

github-actions bot added CORE PYTHON SQL labels Dec 1, 2020

nchammas approved these changes Jan 21, 2021

View changes

apache / spark

[SPARK-33055][PYTHON][SQL] Add Python CalendarIntervalType #29935

[SPARK-33055][PYTHON][SQL] Add Python CalendarIntervalType #29935

zero323 commented Oct 2, 2020 •

edited

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Jan 7, 2021

zero323 commented Jan 16, 2021

MrPowers commented Jan 17, 2021 •

edited

nchammas left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

zero323 commented Jan 21, 2021

		@@ -186,6 +186,30 @@ def fromInternal(self, ts):
		return datetime.datetime.fromtimestamp(ts // 1000000).replace(microsecond=ts % 1000000)


		class CalendarIntervalType(DataType, metaclass=DataTypeSingleton):

apache / spark

[SPARK-33055][PYTHON][SQL] Add Python CalendarIntervalType #29935

[SPARK-33055][PYTHON][SQL] Add Python CalendarIntervalType #29935

Conversation

zero323 commented Oct 2, 2020 • edited

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?

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

SparkQA commented Oct 2, 2020

This comment has been minimized.

HyukjinKwon Oct 3, 2020 Member

This comment has been minimized.

zero323 Oct 3, 2020 Author Contributor

This comment has been minimized.

nchammas Jan 21, 2021 • edited Contributor

This comment has been minimized.

HyukjinKwon Jan 22, 2021 Member

This comment has been minimized.

HyukjinKwon Jan 22, 2021 Member

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Nov 3, 2020

SparkQA commented Jan 7, 2021

zero323 commented Jan 16, 2021

MrPowers commented Jan 17, 2021 • edited

nchammas left a comment

This comment has been minimized.

nchammas Jan 21, 2021 Contributor

This comment has been minimized.

zero323 Jan 21, 2021 Author Contributor

This comment has been minimized.

nchammas Jan 21, 2021 • edited Contributor

zero323 commented Jan 21, 2021

zero323 commented Oct 2, 2020 •

edited

HyukjinKwon Oct 3, 2020
Member

zero323 Oct 3, 2020
Author Contributor

nchammas Jan 21, 2021 •

edited

Contributor

HyukjinKwon Jan 22, 2021
Member

HyukjinKwon Jan 22, 2021
Member

MrPowers commented Jan 17, 2021 •

edited

nchammas Jan 21, 2021
Contributor

zero323 Jan 21, 2021
Author Contributor

nchammas Jan 21, 2021 •

edited

Contributor