You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by WeichenXu123 <gi...@git.apache.org> on 2016/09/14 15:57:13 UTC
[GitHub] spark pull request #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR arra...
GitHub user WeichenXu123 opened a pull request:
https://github.com/apache/spark/pull/15097
[SPARK-17540][SparkR][Spark Core] fix SparkR array serde type problem when length == 0
## What changes were proposed in this pull request?
fix SparkR array serde type problem when length == 0
when length == 0 mark the data type as NULL,
and scala side will check case element type == NULL and generate Array[Nothing] with length == 0
because in scala `Nothing` type is sub-type of any other types,
so that scala Array[Nothing] can be assigned to any variable with type Array[_].
so that this problem can be solved ideally.
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/WeichenXu123/spark fix_r_pass_zero_array_type
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15097.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 #15097
----
commit 08e011f1d38682dcedc93f4ef8b038ee13fbe5cb
Author: WeichenXu <we...@outlook.com>
Date: 2016-09-14T15:14:59Z
update
----
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15097
Merged build finished. Test FAILed.
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15097
**[Test build #65376 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65376/consoleFull)** for PR 15097 at commit [`08e011f`](https://github.com/apache/spark/commit/08e011f1d38682dcedc93f4ef8b038ee13fbe5cb).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/15097
Please add tests for this?
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/15097
Thanks @WeichenXu123 Can we add some test cases for this ? Also was this fix related to any problem you saw ? If so we can also include that in the test cases etc.
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/15097
@felixcheung
it is the case when r-side pass an empty array and scala-side parameter type is Array[T] then there is some problem (as the TODO described in serialize.R: 166 line)...but passing an empty array as an parameter is rarely used, so I consider close this PR for now, thanks!
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/15097
@WeichenXu123 do you have the user code and sample data that when run with SparkR will cause this issue? I think that will help us understand how this happens better.
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15097
**[Test build #65376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65376/consoleFull)** for PR 15097 at commit [`08e011f`](https://github.com/apache/spark/commit/08e011f1d38682dcedc93f4ef8b038ee13fbe5cb).
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/15097
@felixcheung
I think out two ways for this problem, see the PR description.
which is better in your opinion?
Or whether it exists better solution?
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/15097
+1 for adding tests.
---
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 #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR arra...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 closed the pull request at:
https://github.com/apache/spark/pull/15097
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15097
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65376/
Test FAILed.
---
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 issue #15097: [SPARK-17540][SparkR][Spark Core] fix SparkR array serde...
Posted by WeichenXu123 <gi...@git.apache.org>.
Github user WeichenXu123 commented on the issue:
https://github.com/apache/spark/pull/15097
@shivaram
Oh...I found this way still has problems, Array[Nothing] in scala, after compiling with type erasing, at last it turned into type `Ljava.lang.object`, but primitive type Array such as Array[Int] after compile turned into `[I` so the two type still not compatible...
Now I am thinking about another way, when R-side we pass an array with zero length, scala/java side regard it as `null` value, so it can be assigned to any type parameter.
A typical scenario of this problem is, we want to pass an array type parameter from R-side, but sometime we only want to pass `null` array to indicate we want to use the default parameter value.
---
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