Skip to content

[SPARK-24695] [SQL]: To add support to return Calendar interval from udf. - #21679

Closed
priyankagargnitk wants to merge 11 commits into
apache:masterfrom
priyankagargnitk:SPARK-24695
Closed

[SPARK-24695] [SQL]: To add support to return Calendar interval from udf.#21679
priyankagargnitk wants to merge 11 commits into
apache:masterfrom
priyankagargnitk:SPARK-24695

Conversation

@priyankagargnitk

Copy link
Copy Markdown

What changes were proposed in this pull request?

This change adds capability to return Calender interval from udf.

Earlier, the udf of Type (String => CalendarInterval) was throwing Exception stating:
Schema for type org.apache.spark.unsafe.types.CalendarInterval is not supported
java.lang.UnsupportedOperationException: Schema for type org.apache.spark.unsafe.types.CalendarInterval is not supported
at org.apache.spark.sql.catalyst.ScalaReflection391anonfun.apply(ScalaReflection.scala:781)

How was this patch tested?

Added test case in ScalaReflectionSuite.scala and ExpressionEncoderSuite.scala
Also, tested by creating an udf that returns Calendar interval.

jira entry for detail: https://issues.apache.org/jira/browse/SPARK-24695

## What changes were proposed in this pull request?
This change adds capability to return Calender interval from udf.
Earlier, the udf of Type (String => CalendarInterval) was throwing Exception stating:
Schema for type org.apache.spark.unsafe.types.CalendarInterval is not supported
java.lang.UnsupportedOperationException: Schema for type org.apache.spark.unsafe.types.CalendarInterval is not supported
at org.apache.spark.sql.catalyst.ScalaReflection391anonfun.apply(ScalaReflection.scala:781)
## How was this patch tested?
Added test case in ScalaReflectionSuite.scala and ExpressionEncoderSuite.scala
Also, tested by creating an udf that returns Calendar interval.
jira entry for detail: https://issues.apache.org/jira/browse/SPARK-24695
@maropu

Copy link
Copy Markdown
Member

Since CalendarInterval is an internal class, I think users are not intended to use the class directly...

@maropu

Copy link
Copy Markdown
Member

btw, can you update the title like [SPARK-24695][SQL]...

@gatorsmilegatorsmile left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the short term, we do not plan to make CalendarInterval public.

@priyankagargnitkpriyankagargnitk changed the title SPARK-24695: To add support to return Calendar interval from udf.[SPARK-24695] [SQL]: To add support to return Calendar interval from udf.Jul 2, 2018
@priyankagargnitk

priyankagargnitk commented Jul 2, 2018

Copy link
Copy Markdown
Author

org.apache.spark.unsafe.types.CalenderInterval is already public, am i missing something.
Also, what if I want to do some computation on any data type and return Calender Interval... How should I solve this problem in the current scenario?

@maropu

Copy link
Copy Markdown
Member

Actually, the unsafe package does not include user-facing classes.

@HyukjinKwon

Copy link
Copy Markdown
Member

Yea, so it's kind of half public now but not completely exposed. It should be exposed before we extend its support.

@priyankagargnitk

Copy link
Copy Markdown
Author

What if i make changes to expose it?

@HyukjinKwon

Copy link
Copy Markdown
Member

I would better open a discussion thread in the mailing list before making a PR to expose it. Shell we leave this close this meanwhile? I think it's better to leave active and actionable PRs only.

@HyukjinKwon

Copy link
Copy Markdown
Member

I think we should close this for now then.

@wangyumwangyum mentioned this pull request Nov 10, 2018
@gatorsmile

Copy link
Copy Markdown
Member

For your information, @cloud-fan and I are discussing how we expose calendar interval data types. Will post the proposal later.

@hvanhovell

Copy link
Copy Markdown
Contributor

I think this one is reasonable. We already exposed it as an argument to a UDF, so it is pretty poor UX to not allow a return type. As for making CalendarInterval public, what is there to do besides moving it?

Reopening this one.

@hvanhovellhvanhovell reopened this Jun 19, 2019
@hvanhovell

Copy link
Copy Markdown
Contributor

@priyankagargnitk can you update this one.

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@maropu

Copy link
Copy Markdown
Member

@priyankagargnitk ping, are you still there?

@priyankagargnitk

Copy link
Copy Markdown
Author

I am working on this, I'll update the PR in a day or two.

@priyankagargnitk

Copy link
Copy Markdown
Author

A new PR has been raised.

@priyankagargnitk

Copy link
Copy Markdown
Author

#25022

zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
Closesapache#21766Closesapache#21679Closesapache#21161Closesapache#20846Closesapache#19434Closesapache#18080Closesapache#17648Closesapache#17169
Add:
Closesapache#22813Closesapache#21994Closesapache#22005Closesapache#22463
Add:
Closesapache#15899
Add:
Closesapache#22539Closesapache#21868Closesapache#21514Closesapache#21402Closesapache#21322Closesapache#21257Closesapache#20163Closesapache#19691Closesapache#18697Closesapache#18636Closesapache#17176Closesapache#23001 from wangyum/CloseStalePRs.
Authored-by: Yuming Wang <yumwang@ebay.com>
Signed-off-by: hyukjinkwon <gurwls223@apache.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@priyankagargnitk@maropu@HyukjinKwon@gatorsmile@hvanhovell@AmplabJenkins@priyankagar