You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by 10110346 <gi...@git.apache.org> on 2018/08/27 07:34:33 UTC
[GitHub] spark pull request #22241: [SPARK-25249][TEST]add a unit test for OpenHashMa...
GitHub user 10110346 opened a pull request:
https://github.com/apache/spark/pull/22241
[SPARK-25249][TEST]add a unit test for OpenHashMap
## What changes were proposed in this pull request?
This PR adds a unit test for OpenHashMap , this can help developers to distinguish between the 0/0.0/0L and null
## How was this patch tested?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/10110346/spark openhashmap
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22241.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 #22241
----
commit 76b9f48a0b9423cce372fd8b33dbb771c42266b0
Author: liuxian <li...@...>
Date: 2018-08-27T07:23:38Z
add unit test for OpenHashMap
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95282/testReport)** for PR 22241 at commit [`76b9f48`](https://github.com/apache/spark/commit/76b9f48a0b9423cce372fd8b33dbb771c42266b0).
* 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 #22241: [SPARK-25249][CORE][TEST]add a unit test for Open...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22241#discussion_r212902991
--- Diff: core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala ---
@@ -194,4 +194,42 @@ class OpenHashMapSuite extends SparkFunSuite with Matchers {
val numInvalidValues = map.iterator.count(_._2 == 0)
assertResult(0)(numInvalidValues)
}
+
+ test("distinguish between the 0/0.0/0L and null") {
+ val specializedMap1 = new OpenHashMap[String, Long]
+ specializedMap1("a") = null.asInstanceOf[Long]
+ specializedMap1("b") = 0L
+ assert(specializedMap1.contains("a"))
+ assert(!specializedMap1.contains("c"))
+ assert(Some(specializedMap1("a")).contains(0L))
+ assert(Some(specializedMap1("b")).contains(0L))
+ assert(Some(specializedMap1("c")).contains(0L))
--- End diff --
ok , i will add it
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95282/
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 #22241: [SPARK-25249][TEST]add a unit test for OpenHashMa...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/22241#discussion_r212894122
--- Diff: core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala ---
@@ -194,4 +194,42 @@ class OpenHashMapSuite extends SparkFunSuite with Matchers {
val numInvalidValues = map.iterator.count(_._2 == 0)
assertResult(0)(numInvalidValues)
}
+
+ test("distinguish between the 0/0.0/0L and null") {
+ val specializedMap1 = new OpenHashMap[String, Long]
+ specializedMap1("a") = null.asInstanceOf[Long]
+ specializedMap1("b") = 0L
+ assert(specializedMap1.contains("a"))
+ assert(!specializedMap1.contains("c"))
+ assert(Some(specializedMap1("a")).contains(0L))
+ assert(Some(specializedMap1("b")).contains(0L))
+ assert(Some(specializedMap1("c")).contains(0L))
--- End diff --
What do these tests mean? Tests for different type comparisons? e.g.,
```
assert(!Some(specializedMap1("a")).contains(0.0))
assert(!Some(specializedMap1("b")).contains(0.toShort))
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2568/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22241
Adding tests looks good.
Probably beyond the purpose of this PR, is the following difference intentionally designed?
```
// If the data type is in @specialized annotation, and
// the `key` is not be contained, the `map(key)` will return 0
```
```
// If the data type is not in @specialized annotation, and
// the `key` is not be contained, the `map(key)` will return null
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22241
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22241: [SPARK-25249][CORE][TEST]add a unit test for Open...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/22241#discussion_r212901257
--- Diff: core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala ---
@@ -194,4 +194,42 @@ class OpenHashMapSuite extends SparkFunSuite with Matchers {
val numInvalidValues = map.iterator.count(_._2 == 0)
assertResult(0)(numInvalidValues)
}
+
+ test("distinguish between the 0/0.0/0L and null") {
+ val specializedMap1 = new OpenHashMap[String, Long]
+ specializedMap1("a") = null.asInstanceOf[Long]
+ specializedMap1("b") = 0L
+ assert(specializedMap1.contains("a"))
+ assert(!specializedMap1.contains("c"))
+ assert(Some(specializedMap1("a")).contains(0L))
+ assert(Some(specializedMap1("b")).contains(0L))
+ assert(Some(specializedMap1("c")).contains(0L))
--- End diff --
ah, I see. So, can you describe more in the description?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2570/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95282/testReport)** for PR 22241 at commit [`76b9f48`](https://github.com/apache/spark/commit/76b9f48a0b9423cce372fd8b33dbb771c42266b0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95285 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95285/testReport)** for PR 22241 at commit [`8aea12e`](https://github.com/apache/spark/commit/8aea12ef32b6eb43f2e23963ef18e9abb2c05b53).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22241: [SPARK-25249][TEST]add a unit test for OpenHashMa...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22241#discussion_r212897158
--- Diff: core/src/test/scala/org/apache/spark/util/collection/OpenHashMapSuite.scala ---
@@ -194,4 +194,42 @@ class OpenHashMapSuite extends SparkFunSuite with Matchers {
val numInvalidValues = map.iterator.count(_._2 == 0)
assertResult(0)(numInvalidValues)
}
+
+ test("distinguish between the 0/0.0/0L and null") {
+ val specializedMap1 = new OpenHashMap[String, Long]
+ specializedMap1("a") = null.asInstanceOf[Long]
+ specializedMap1("b") = 0L
+ assert(specializedMap1.contains("a"))
+ assert(!specializedMap1.contains("c"))
+ assert(Some(specializedMap1("a")).contains(0L))
+ assert(Some(specializedMap1("b")).contains(0L))
+ assert(Some(specializedMap1("c")).contains(0L))
--- End diff --
if the data type is in `@specialized annotation` , and the `key` not be contained ,the `map(key)` will return 0
if the data type is not in `@specialized annotation` , and the `key` not be contained ,the `map(key)` will return null
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22241
thanks @maropu
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95286/testReport)** for PR 22241 at commit [`9346112`](https://github.com/apache/spark/commit/9346112c8e94b4755ae07ea926997acc7b595937).
* 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 #22241: [SPARK-25249][TEST]add a unit test for OpenHashMap
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/22241
plz add `[CORE]` in the title?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95286/testReport)** for PR 22241 at commit [`9346112`](https://github.com/apache/spark/commit/9346112c8e94b4755ae07ea926997acc7b595937).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95286/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22241
**[Test build #95285 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95285/testReport)** for PR 22241 at commit [`8aea12e`](https://github.com/apache/spark/commit/8aea12ef32b6eb43f2e23963ef18e9abb2c05b53).
* 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 #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95285/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2571/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22241
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 #22241: [SPARK-25249][CORE][TEST]add a unit test for Open...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22241
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22241: [SPARK-25249][CORE][TEST]add a unit test for OpenHashMap
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22241
@kiszk I guess it's because in this case the underlying value type is a primitive like int or long, so null can't be returned?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org