You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by baishuo <gi...@git.apache.org> on 2015/04/23 12:09:43 UTC
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
GitHub user baishuo opened a pull request:
https://github.com/apache/spark/pull/5660
[SPARK-6505][SQL]Remove the reflection call in HiveFunctionWrapper
according @liancheng‘s comment in https://issues.apache.org/jira/browse/SPARK-6505, this patch remove the reflection call in HiveFunctionWrapper, and implement the functions named "deserializeObjectByKryo" and "serializeObjectByKryo" according the functions with the save name in
org.apache.hadoop.hive.ql.exec.Utilities.java
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/baishuo/spark SPARK-6505-20150423
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/5660.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5660
----
commit a5ff9c7c90b89288822c90474d8f01dd13743c8d
Author: baishuo <vc...@hotmail.com>
Date: 2015-04-23T09:54:17Z
Remove the reflection call in HiveFunctionWrapper
commit 0b522a77ffe83889e7c6afaa963dd49e5675fe36
Author: baishuo <vc...@hotmail.com>
Date: 2015-04-23T09:59:48Z
modify code style
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95779919
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30892/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5660#discussion_r28974373
--- Diff: sql/hive/v0.13.1/src/main/scala/org/apache/spark/sql/hive/Shim13.scala ---
@@ -61,39 +62,40 @@ private[hive] case class HiveFunctionWrapper(var functionClassName: String)
// for Serialization
def this() = this(null)
+
+ import java.io.{OutputStream, InputStream}
+
+ import com.esotericsoftware.kryo.io.Input
+ import com.esotericsoftware.kryo.io.Output
+
import org.apache.spark.util.Utils._
@transient
- private val methodDeSerialize = {
- val method = classOf[Utilities].getDeclaredMethod(
- "deserializeObjectByKryo",
- classOf[Kryo],
- classOf[java.io.InputStream],
- classOf[Class[_]])
- method.setAccessible(true)
-
- method
+ private def deserializeObjectByKryo[T: ClassTag](kryo: Kryo,
+ in: InputStream,
+ clazz: Class[_]): T = {
+ val inp = new Input(in)
+ val t: T = kryo.readObject(inp,clazz).asInstanceOf[T]
+ inp.close()
+ t
}
@transient
- private val methodSerialize = {
- val method = classOf[Utilities].getDeclaredMethod(
- "serializeObjectByKryo",
- classOf[Kryo],
- classOf[Object],
- classOf[java.io.OutputStream])
- method.setAccessible(true)
-
- method
+ private def serializeObjectByKryo(kryo: Kryo,
+ plan: Object,
+ out: OutputStream ) {
--- End diff --
Same as above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95658372
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30848/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/5660
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95523973
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95661038
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/30849/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95627460
Would like to add some background about this change:
Spark SQL repackaged Hive jars to remove unnecessary shading and dependencies. One of them is Kryo. Hive shades Kryo to package `org.apache.hive.com.esotericsoftware`. Normally this should be fine. However, MapR replaces Spark SQL's repackaged Hive dependencies with genuine Hive packages, thus the reflection call mentioned in this PR couldn't find Kryo because it's moved into another package. On the other hand, the two reflected methods are actually quite simple. That's why we chose to reimplement them in Spark SQL instead of using them via reflection.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95626620
[Test build #30848 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30848/consoleFull) for PR 5660 at commit [`0b522a7`](https://github.com/apache/spark/commit/0b522a77ffe83889e7c6afaa963dd49e5675fe36).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-97766472
@baishuo what's your Apache JIRA username? I can assign you to the JIRA to credit you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95661012
[Test build #30849 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30849/consoleFull) for PR 5660 at commit [`0b522a7`](https://github.com/apache/spark/commit/0b522a77ffe83889e7c6afaa963dd49e5675fe36).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
* This patch does not change any dependencies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95629047
[Test build #30849 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30849/consoleFull) for PR 5660 at commit [`0b522a7`](https://github.com/apache/spark/commit/0b522a77ffe83889e7c6afaa963dd49e5675fe36).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5660#discussion_r28974357
--- Diff: sql/hive/v0.13.1/src/main/scala/org/apache/spark/sql/hive/Shim13.scala ---
@@ -61,39 +62,40 @@ private[hive] case class HiveFunctionWrapper(var functionClassName: String)
// for Serialization
def this() = this(null)
+
+ import java.io.{OutputStream, InputStream}
+
+ import com.esotericsoftware.kryo.io.Input
+ import com.esotericsoftware.kryo.io.Output
+
import org.apache.spark.util.Utils._
@transient
- private val methodDeSerialize = {
- val method = classOf[Utilities].getDeclaredMethod(
- "deserializeObjectByKryo",
- classOf[Kryo],
- classOf[java.io.InputStream],
- classOf[Class[_]])
- method.setAccessible(true)
-
- method
+ private def deserializeObjectByKryo[T: ClassTag](kryo: Kryo,
+ in: InputStream,
+ clazz: Class[_]): T = {
--- End diff --
Don't indent method arguments in this style. Spark uses the following style:
```
def methodName(
arg1: Type1,
arg2: Type2): ReturnType = {
// ...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95658337
[Test build #30848 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30848/consoleFull) for PR 5660 at commit [`0b522a7`](https://github.com/apache/spark/commit/0b522a77ffe83889e7c6afaa963dd49e5675fe36).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
* This patch does not change any dependencies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95625891
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95628037
add to whitelist
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by baishuo <gi...@git.apache.org>.
Github user baishuo commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-96318903
can this patch be merged? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/5660#discussion_r28974257
--- Diff: sql/hive/v0.13.1/src/main/scala/org/apache/spark/sql/hive/Shim13.scala ---
@@ -61,39 +62,40 @@ private[hive] case class HiveFunctionWrapper(var functionClassName: String)
// for Serialization
def this() = this(null)
+
+ import java.io.{OutputStream, InputStream}
+
+ import com.esotericsoftware.kryo.io.Input
+ import com.esotericsoftware.kryo.io.Output
--- End diff --
Move imports to the header of the file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-96515766
Thanks for working on this! I'm merging this to master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95779918
[Test build #30892 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30892/consoleFull) for PR 5660 at commit [`ae61ec4`](https://github.com/apache/spark/commit/ae61ec408bfd7e85fa3dac5b2da368767e748136).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
* This patch does not change any dependencies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-6505][SQL]Remove the reflection call in...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/5660#issuecomment-95765632
[Test build #30892 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/30892/consoleFull) for PR 5660 at commit [`ae61ec4`](https://github.com/apache/spark/commit/ae61ec408bfd7e85fa3dac5b2da368767e748136).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org