Skip to content

Conversation

@gatorsmile
Copy link
Member

@gatorsmile gatorsmile commented Feb 12, 2018

What changes were proposed in this pull request?

In this upcoming 2.3 release, we changed the interface of ScalaUDF. Unfortunately, some Spark packages (e.g., spark-deep-learning) are using our internal class ScalaUDF. In the release 2.3, we added new parameters into this class. The users hit the binary compatibility issues and got the exception:

> java.lang.NoSuchMethodError: org.apache.spark.sql.catalyst.expressions.ScalaUDF.<init>(Ljava/lang/Object;Lorg/apache/spark/sql/types/DataType;Lscala/collection/Seq;Lscala/collection/Seq;Lscala/Option;)V

This PR is to improve the backward compatibility. However, we definitely should not encourage the external packages to use our internal classes. This might make us hard to maintain/develop the codes in Spark.

How was this patch tested?

N/A

@gatorsmile
Copy link
Member Author

cc @cloud-fan @hvanhovell @zsxwing

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87352 has finished for PR 20591 at commit 2b6be52.

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

@cloud-fan
Copy link
Contributor

The JIRA ID is wrong...

@gatorsmile gatorsmile changed the title [SPARK-23390] [SQL] Add two extra constructors for ScalaUDF [SPARK-23400] [SQL] Add two extra constructors for ScalaUDF Feb 13, 2018
@gatorsmile gatorsmile changed the title [SPARK-23400] [SQL] Add two extra constructors for ScalaUDF [SPARK-23400] [SQL] Add a constructors for ScalaUDF Feb 13, 2018
@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87363 has finished for PR 20591 at commit 08b39d0.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87380 has finished for PR 20591 at commit 08b39d0.

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

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Feb 13, 2018

Test build #87390 has finished for PR 20591 at commit 08b39d0.

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

@zsxwing
Copy link
Member

zsxwing commented Feb 13, 2018

LGTM. Merging to master and 2.3.

asfgit pushed a commit that referenced this pull request Feb 13, 2018
## What changes were proposed in this pull request?

In this upcoming 2.3 release, we changed the interface of `ScalaUDF`. Unfortunately, some Spark packages (e.g., spark-deep-learning) are using our internal class `ScalaUDF`. In the release 2.3, we added new parameters into this class. The users hit the binary compatibility issues and got the exception:

```
> java.lang.NoSuchMethodError: org.apache.spark.sql.catalyst.expressions.ScalaUDF.<init>(Ljava/lang/Object;Lorg/apache/spark/sql/types/DataType;Lscala/collection/Seq;Lscala/collection/Seq;Lscala/Option;)V
```

This PR is to improve the backward compatibility. However, we definitely should not encourage the external packages to use our internal classes. This might make us hard to maintain/develop the codes in Spark.

## How was this patch tested?
N/A

Author: gatorsmile <[email protected]>

Closes #20591 from gatorsmile/scalaUDF.

(cherry picked from commit 2ee76c2)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in 2ee76c2 Feb 13, 2018
Sign up for free to 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.

4 participants