[SPARK-20416][SQL] Print UDF names in EXPLAIN #17712

maropu · 2017-04-21T03:09:30Z

What changes were proposed in this pull request?

This pr added withName in UserDefinedFunction for printing UDF names in EXPLAIN

How was this patch tested?

Added tests in UDFSuite.

SparkQA · 2017-04-21T05:22:06Z

Test build #76013 has finished for PR 17712 at commit 90c516f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2017-04-21T05:27:00Z

cc: @gatorsmile

rxin · 2017-04-21T07:04:07Z

sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala

can we create a new instance instead so this is immutable?

okay, I'll fix

Beside @rxin's comment, I'd also add name as an input parameter with default value None.

@jaceklaskowski sorry, but I missed your point. Could you give more more?

@rxin I tried to make this immutable though, IIUC this is no easy & simple way to do that... any idea?

@maropu I guess @jaceklaskowski wants to make this like:

case class UserDefinedFunction protected[sql] ( f: AnyRef, dataType: DataType, inputTypes: Option[Seq[DataType]], name: Option[String] = None) { ... }

yea, but, the addition affects binary compatibility?

it will be fine if we add an explicit apply method and unapply method.

okay, I'll recheck

jaceklaskowski · 2017-04-21T08:11:45Z

sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala

Remove { and } (as they're not needed)

jaceklaskowski · 2017-04-21T08:13:46Z

sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala

The goal of the change was to make sure that the names are the same for SQL and Dataset "modes". The test should check it (even though it does it using the above two tests the last one should rather check equality of SQL's and Dataset's outputs).

SparkQA · 2017-04-21T08:37:46Z

Test build #76024 has finished for PR 17712 at commit 9cececb.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UserDefinedFunctionWithName protected[sql] (

SparkQA · 2017-04-21T08:47:58Z

Test build #76025 has finished for PR 17712 at commit 9ab4302.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-21T09:03:04Z

Test build #76026 has finished for PR 17712 at commit 1a1d436.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-21T11:06:42Z

Test build #76027 has finished for PR 17712 at commit b08154e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

jaceklaskowski · 2017-04-21T13:56:22Z

sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala

Can a UserDefinedFunctionWithName have no name? When?

jaceklaskowski · 2017-04-21T13:58:01Z

sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala

"Optionally"? I'd assume the old "UDF" is the default unless the name is given.

SparkQA · 2017-04-22T00:37:21Z

Test build #76050 has finished for PR 17712 at commit 5d797a9.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-22T01:17:28Z

Test build #76052 has finished for PR 17712 at commit 96bc89d.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2017-04-22T02:28:07Z

sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala

also need an unapply function

ok. Is it okay to update the MiMa file?

oh, it seems we couldn't simply add unapply there because it conflicts with the unapply implicitly generated by the case class:

[error] /Users/maropu/IdeaProjects/spark/spark-master/sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFunction.scala:45: method unapply is defined twic e [error] conflicting symbols both originated in file '/Users/maropu/IdeaProjects/spark/spark-master/sql/core/src/main/scala/org/apache/spark/sql/expressions/UserDefinedFu nction.scala' [error] case class UserDefinedFunction protected[sql] ( [error] ^

ah ok - that sucks. that means this will break compatibility ...

for now, I'll revert it...

SparkQA · 2017-04-22T03:02:25Z

Test build #76053 has finished for PR 17712 at commit 8800c3b.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-04-22T05:57:32Z

Test build #76057 has started for PR 17712 at commit dd182e4.

maropu · 2017-04-22T08:24:03Z

Jenkins, retest this please.

SparkQA · 2017-04-22T10:36:27Z

Test build #76059 has finished for PR 17712 at commit dd182e4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2017-04-22T18:37:42Z

cc @gatorsmile

This is related to the deterministic thing you want to do?

gatorsmile · 2017-04-23T06:40:37Z

Yes! My PR has not been submitted due to my family issues. In addition to the name and deterministic flag, we have another two Scala UDF properties based on the existing Hive UDF types. Instead of adding them one by one, we plan to use a Map.

maropu · 2017-04-23T06:53:15Z

Aha, good! We already have a related JIRA ticket for that? I'ld like to leave this issue to it.

rxin · 2017-04-23T18:14:45Z

Why use a map? That's super unstructured and easy to break ...

maropu · 2017-04-26T23:28:28Z

@gatorsmile WDYT? If this pr possibly merged, I still open; otherwise I'll close.

gatorsmile · 2017-05-03T22:51:46Z

Sorry for the delay, just submitted a PR for addressing the related issues. The PR fixed the issue using a different way. Could you review that PR? #17848

Thanks!

SparkQA · 2017-05-11T11:17:44Z

Test build #76799 has finished for PR 17712 at commit 821f47d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-05-11T14:36:54Z

Test build #76807 has finished for PR 17712 at commit 61bd96c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-05-11T16:47:51Z

LGTM

gatorsmile · 2017-05-11T16:48:29Z

Thanks! Merging to master.

## What changes were proposed in this pull request? This pr added `withName` in `UserDefinedFunction` for printing UDF names in EXPLAIN ## How was this patch tested? Added tests in `UDFSuite`. Author: Takeshi Yamamuro <[email protected]> Closes apache#17712 from maropu/SPARK-20416.

rxin reviewed Apr 21, 2017

View reviewed changes

jaceklaskowski reviewed Apr 21, 2017

View reviewed changes

maropu force-pushed the SPARK-20416 branch from 5d797a9 to 96bc89d Compare April 22, 2017 01:04

rxin reviewed Apr 22, 2017

View reviewed changes

maropu force-pushed the SPARK-20416 branch from 96bc89d to 8800c3b Compare April 22, 2017 02:52

maropu force-pushed the SPARK-20416 branch from dd182e4 to 821f47d Compare May 11, 2017 09:44

maropu force-pushed the SPARK-20416 branch from 821f47d to 383390f Compare May 11, 2017 12:23

Print UDF names in Dataset APIs

61bd96c

maropu force-pushed the SPARK-20416 branch from 383390f to 61bd96c Compare May 11, 2017 12:23

asfgit closed this in 3aa4e46 May 11, 2017

[SPARK-20416][SQL] Print UDF names in EXPLAIN #17712

[SPARK-20416][SQL] Print UDF names in EXPLAIN #17712

Uh oh!

Conversation

maropu commented Apr 21, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Apr 21, 2017

Uh oh!

maropu commented Apr 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 21, 2017

Uh oh!

SparkQA commented Apr 21, 2017

Uh oh!

SparkQA commented Apr 21, 2017

Uh oh!

SparkQA commented Apr 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 22, 2017

Uh oh!

SparkQA commented Apr 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu Apr 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 22, 2017

Uh oh!

SparkQA commented Apr 22, 2017

Uh oh!

maropu commented Apr 22, 2017

Uh oh!

SparkQA commented Apr 22, 2017

Uh oh!

rxin commented Apr 22, 2017

Uh oh!

gatorsmile commented Apr 23, 2017

Uh oh!

maropu commented Apr 23, 2017

Uh oh!

rxin commented Apr 23, 2017

Uh oh!

maropu commented Apr 26, 2017

Uh oh!

maropu Apr 22, 2017 •

edited

Loading

gatorsmile commented May 3, 2017 •

edited

Loading