You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by fangshil <gi...@git.apache.org> on 2018/05/09 00:16:39 UTC
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
GitHub user fangshil opened a pull request:
https://github.com/apache/spark/pull/21276
[SPARK-24216][SQL] Spark TypedAggregateExpression uses getSimpleName that is not safe in scala
## What changes were proposed in this pull request?
When we create a aggregator object within a function in scala and pass the aggregator to Spark Dataset's aggregation method, Spark's will initialize TypedAggregateExpression with the name field as aggregator.getClass.getSimpleName. However, getSimpleName is not safe in scala environment, for example, if the aggregator class full qualified name is "com.linkedin.spark.common.lib.SparkUtils$keyAgg$2$", the getSimpleName will throw exception "Malformed class name". This has been reported in scalatest https://github.com/scalatest/scalatest/pull/1044 and scala upstream jira https://issues.scala-lang.org/browse/SI-8110.
To fix this issue, we follow the solution in https://github.com/scalatest/scalatest/pull/1044 to add safer version of getSimpleName as a util method, and TypedAggregateExpression will invoke this util method rather than getClass.getSimpleName.
## How was this patch tested?
added unit test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/fangshil/spark SPARK-24216
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21276.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 #21276
----
commit a43493b6299d4b6962f2b78c6956b29df51a78c9
Author: Fangshi Li <fl...@...>
Date: 2018-04-27T19:54:49Z
SPARK-24216: Spark TypedAggregateExpression uses getSimpleName that is not safe in scala
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187278875
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2715,66 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * A safer version than scala obj's getClass.getSimpleName and Utils.getFormattedClassName
+ * which may throw Malformed class name error.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(fullyQualifiedName: String): String = {
+ stripDollars(parseSimpleName(fullyQualifiedName))
+ }
+
+ /**
+ * Remove the packages from full qualified class name
+ */
+ private def parseSimpleName(fullyQualifiedName: String): String = {
--- End diff --
`fullyQualifiedName.split("\\.").takeRight(1)`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90407/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90451 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90451/testReport)** for PR 21276 at commit [`d0edc81`](https://github.com/apache/spark/commit/d0edc813b1cbb785cf15e7d471028a8b71167aa3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187271712
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,32 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ object A {
--- End diff --
updated
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on the issue:
https://github.com/apache/spark/pull/21276
made a few changes:
since getSimpleName is a method of Class, it makes sense for Utils.getSimpleName to takes a Class param
Utils.getSimpleName will try to use the Class's getSimpleName first, and only use the alternative if the internal error is caught. This should minimize the potential impact of this patch
updated a few other places where similar issue may happen
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on the issue:
https://github.com/apache/spark/pull/21276
>> Do we have other places in Spark calling getSimpleName which also need this change?
there are a few other places that may have similar issue. will update together
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90445 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90445/testReport)** for PR 21276 at commit [`3fbb93e`](https://github.com/apache/spark/commit/3fbb93ee456720dabdc9f38628943ce7392e061e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90498/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90445/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on the issue:
https://github.com/apache/spark/pull/21276
updating
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91609/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90418/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21276
@gatorsmile @hvanhovell could you trigger tests?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187279829
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,35 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ object MalformedClassObject {
+ class MalformedClass
+ }
+
+ test("Safe getSimpleName") {
+ val fullname1 = "org.apache.spark.TestClass$MyClass"
+ assert(Utils.getSimpleName(fullname1) === "TestClass$MyClass")
+
+ val fullname2 = "org.apache.spark.TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname2) === "TestClass$MyClass")
+
+ val fullname3 = "org.apache.spark.TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname3) === "TestClass$MyClass$1")
+
+ val fullname4 = "TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname4) === "TestClass$MyClass$1")
+
+ val fullname5 = "$iwC$iwC$$iwC$$iwC$TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname5) === "MyClass")
+
+ // getSimpleName on class MalformedClass will result in error: Malformed class name
+ // Utils.getSimpleName works
+ intercept[java.lang.InternalError] {
+ classOf[MalformedClassObject.MalformedClass].getSimpleName
+ }
--- End diff --
Can we check the err message?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187282260
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2715,66 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * A safer version than scala obj's getClass.getSimpleName and Utils.getFormattedClassName
+ * which may throw Malformed class name error.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(fullyQualifiedName: String): String = {
--- End diff --
How about changing the method signature into `def getSimpleName(obj: AnyRef)`?
Then, we simply handle two cases; one is correct and the other non-correct?, e.g.,
```
def getSimpleName(obj: AnyRef): String = {
if (incorrect case) {
// Canonicalizes the name for correction
...
} else {
// If no problem, just returns getSimplename
obj.getClass.getSimpleName
}
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/21276
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #91609 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91609/testReport)** for PR 21276 at commit [`3d067b8`](https://github.com/apache/spark/commit/3d067b883a947b0b3b3dd200c7767125001aefef).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90449 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90449/testReport)** for PR 21276 at commit [`769ad7d`](https://github.com/apache/spark/commit/769ad7d2bcd43a97f8622300b42a4acbdd7f63db).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90446/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90407 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90407/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21276
How about adding a `sql` method in `Aggregator` and printing it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187263297
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TypedAggregateExpression.scala ---
@@ -109,7 +110,8 @@ trait TypedAggregateExpression extends AggregateFunction {
s"$nodeName($input)"
}
- override def nodeName: String = aggregator.getClass.getSimpleName.stripSuffix("$")
+ // Utils.getSimpleName is safer than aggregator.getClass.getSimpleName in scala.
--- End diff --
updated comments
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r186983813
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TypedAggregateExpression.scala ---
@@ -109,7 +110,8 @@ trait TypedAggregateExpression extends AggregateFunction {
s"$nodeName($input)"
}
- override def nodeName: String = aggregator.getClass.getSimpleName.stripSuffix("$")
+ // Utils.getSimpleName is safer than aggregator.getClass.getSimpleName in scala.
--- End diff --
```aggregator.getClass.getSimpleName can cause Malformed class name error, call safer `Utils.getSimpleName` instead. ```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90524/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90449/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90407 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90407/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90525 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90525/testReport)** for PR 21276 at commit [`3d067b8`](https://github.com/apache/spark/commit/3d067b883a947b0b3b3dd200c7767125001aefef).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90410 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90410/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21276
this is a safe fix, I'm also merging it to 2.3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/21276
@fangshil Can you update?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90446 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90446/testReport)** for PR 21276 at commit [`e286732`](https://github.com/apache/spark/commit/e286732fe395b5b4ac7989d6f57d8b6effca81cf).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/21276
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/21276
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90410/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90498 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90498/testReport)** for PR 21276 at commit [`70b3c81`](https://github.com/apache/spark/commit/70b3c812a59a758c517bb7bc60704712ac589c9e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90525/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187540472
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,35 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ object MalformedClassObject {
+ class MalformedClass
+ }
+
+ test("Safe getSimpleName") {
+ val fullname1 = "org.apache.spark.TestClass$MyClass"
+ assert(Utils.getSimpleName(fullname1) === "TestClass$MyClass")
+
+ val fullname2 = "org.apache.spark.TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname2) === "TestClass$MyClass")
+
+ val fullname3 = "org.apache.spark.TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname3) === "TestClass$MyClass$1")
+
+ val fullname4 = "TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname4) === "TestClass$MyClass$1")
+
+ val fullname5 = "$iwC$iwC$$iwC$$iwC$TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname5) === "MyClass")
+
+ // getSimpleName on class MalformedClass will result in error: Malformed class name
+ // Utils.getSimpleName works
+ intercept[java.lang.InternalError] {
+ classOf[MalformedClassObject.MalformedClass].getSimpleName
+ }
--- End diff --
updated
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90446 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90446/testReport)** for PR 21276 at commit [`e286732`](https://github.com/apache/spark/commit/e286732fe395b5b4ac7989d6f57d8b6effca81cf).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90418/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90418 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90418/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/21276
thanks, merging to master!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90498 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90498/testReport)** for PR 21276 at commit [`70b3c81`](https://github.com/apache/spark/commit/70b3c812a59a758c517bb7bc60704712ac589c9e).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90445 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90445/testReport)** for PR 21276 at commit [`3fbb93e`](https://github.com/apache/spark/commit/3fbb93ee456720dabdc9f38628943ce7392e061e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #91609 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91609/testReport)** for PR 21276 at commit [`3d067b8`](https://github.com/apache/spark/commit/3d067b883a947b0b3b3dd200c7767125001aefef).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90524 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90524/testReport)** for PR 21276 at commit [`9ff048a`](https://github.com/apache/spark/commit/9ff048ab08b55e48fbcf7acd7672e3205b46b9aa).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r186984569
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,23 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ test("Test that Utils.getSimpleName works as expected") {
+ val fullname1 = "org.apache.spark.TestClass$MyClass"
+ assert(Utils.getSimpleName(fullname1) === "TestClass$MyClass")
+
+ val fullname2 = "org.apache.spark.TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname2) === "TestClass$MyClass")
+
+ val fullname3 = "org.apache.spark.TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname3) === "TestClass$MyClass$1")
+
+ val fullname4 = "TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname4) === "TestClass$MyClass$1")
+
+ val fullname5 = "$iwC$iwC$$iwC$$iwC$TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname5) === "MyClass")
--- End diff --
Is it possible to have a case which causes Malformed class name error?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187264126
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,32 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ object A {
--- End diff --
nit: more meaningful name, e.g.:
```scala
object MalformedClassObect {
class MalformedClass
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187540452
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2715,66 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * A safer version than scala obj's getClass.getSimpleName and Utils.getFormattedClassName
+ * which may throw Malformed class name error.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(fullyQualifiedName: String): String = {
+ stripDollars(parseSimpleName(fullyQualifiedName))
+ }
+
+ /**
+ * Remove the packages from full qualified class name
+ */
+ private def parseSimpleName(fullyQualifiedName: String): String = {
--- End diff --
this method was copied from scalatest. I think it makes sense to update in this cleaner way
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187263253
--- Diff: core/src/test/scala/org/apache/spark/util/UtilsSuite.scala ---
@@ -1168,6 +1168,23 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties with Logging {
Utils.checkAndGetK8sMasterUrl("k8s://foo://host:port")
}
}
+
+ test("Test that Utils.getSimpleName works as expected") {
+ val fullname1 = "org.apache.spark.TestClass$MyClass"
+ assert(Utils.getSimpleName(fullname1) === "TestClass$MyClass")
+
+ val fullname2 = "org.apache.spark.TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname2) === "TestClass$MyClass")
+
+ val fullname3 = "org.apache.spark.TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname3) === "TestClass$MyClass$1")
+
+ val fullname4 = "TestClass$MyClass$1$"
+ assert(Utils.getSimpleName(fullname4) === "TestClass$MyClass$1")
+
+ val fullname5 = "$iwC$iwC$$iwC$$iwC$TestClass$MyClass$"
+ assert(Utils.getSimpleName(fullname5) === "MyClass")
--- End diff --
updated the test to add a case that shows Class's getSimpleName throws Malformed class name error and the Utils.getSimpleName works
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90524 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90524/testReport)** for PR 21276 at commit [`9ff048a`](https://github.com/apache/spark/commit/9ff048ab08b55e48fbcf7acd7672e3205b46b9aa).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187540911
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2715,66 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * A safer version than scala obj's getClass.getSimpleName and Utils.getFormattedClassName
+ * which may throw Malformed class name error.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(fullyQualifiedName: String): String = {
--- End diff --
updated. since getSimpleName is a method from Class, it makes sense for Utils.getSimpleName to also takes a Class param. Also, Utils.getSimpleName will try to use the Class's getSimpleName first, and only use the alternative if the internal error is caught. This should minimize the potential impact of this patch
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90451 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90451/testReport)** for PR 21276 at commit [`d0edc81`](https://github.com/apache/spark/commit/d0edc813b1cbb785cf15e7d471028a8b71167aa3).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21276
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r194630048
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2716,62 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * Safer than Class obj's getSimpleName which may throw Malformed class name error in scala.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(cls: Class[_]): String = {
+ try {
+ return cls.getSimpleName
+ } catch {
+ case err: InternalError => return stripDollars(stripPackages(cls.getName))
+ }
+ }
+
+ /**
+ * Remove the packages from full qualified class name
+ */
+ private def stripPackages(fullyQualifiedName: String): String = {
+ fullyQualifiedName.split("\\.").takeRight(1)(0)
+ }
+
+ /**
+ * Remove trailing dollar signs from qualified class name,
+ * and return the trailing part after the last dollar sign in the middle
+ */
+ private def stripDollars(s: String): String = {
+ val lastDollarIndex = s.lastIndexOf('$')
+ if (lastDollarIndex < s.length - 1) {
+ // The last char is not a dollar sign
+ if (lastDollarIndex == -1 || !s.contains("$iw")) {
+ // The name does not have dollar sign or is not an intepreter
+ // generated class, so we should return the full string
+ s
+ } else {
+ // The class name is intepreter generated,
+ // return the part after the last dollar sign
+ // This is the same behavior as getClass.getSimpleName
+ s.substring(lastDollarIndex + 1)
+ }
+ }
+ else {
--- End diff --
style:
```scala
if (...) {
} else {
}
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90525 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90525/testReport)** for PR 21276 at commit [`3d067b8`](https://github.com/apache/spark/commit/3d067b883a947b0b3b3dd200c7767125001aefef).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90451/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90449/testReport)** for PR 21276 at commit [`769ad7d`](https://github.com/apache/spark/commit/769ad7d2bcd43a97f8622300b42a4acbdd7f63db).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression...
Posted by fangshil <gi...@git.apache.org>.
Github user fangshil commented on a diff in the pull request:
https://github.com/apache/spark/pull/21276#discussion_r187518454
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -2715,6 +2715,66 @@ private[spark] object Utils extends Logging {
HashCodes.fromBytes(secretBytes).toString()
}
+ /**
+ * A safer version than scala obj's getClass.getSimpleName and Utils.getFormattedClassName
+ * which may throw Malformed class name error.
+ * This method mimicks scalatest's getSimpleNameOfAnObjectsClass.
+ */
+ def getSimpleName(fullyQualifiedName: String): String = {
--- End diff --
a if statement to determine if the name is good or not may not be stable and comprehensive - how about we use try-catch here and only use the alternative if obj.getClass.getSimpleName throws the Malformed class name error
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21276
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21276
**[Test build #90410 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90410/testReport)** for PR 21276 at commit [`a43493b`](https://github.com/apache/spark/commit/a43493b6299d4b6962f2b78c6956b29df51a78c9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/21276
I think this fixing is nice to have. cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21276: [SPARK-24216][SQL] Spark TypedAggregateExpression uses g...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/21276
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org