Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/9222
---
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 enab
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151305314
Ok I see -- we used to make a Seq[Array[Any]] and then call toArray on it
before and now we just have Array[Array[Any]]. Alright change LGTM. Merging this
---
If your
Github user FRosner commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151293332
@felixcheung the runtime performance tests I conducted came to the result
that the performance gain is minimal if there is enough RAM available. It was
faster all the ti
Github user FRosner commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151236579
In my opinion we are good to go if there are no objections from your side.
--
Sent from my phone. Please excuse my brevity.
Shivaram Venkataraman wrote
Github user felixcheung commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151232905
@FRosner I assume this is faster than the original implementation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on Gi
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151194783
The class is already `private[r]` so it should be fine.
@FRosner Is the change good to go or are you doing any more perf tests ?
---
If your project is set up
Github user sun-rui commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151060081
@FRosner, I am not denying a unit test:). My point is that it seems not
necessary to add a Scala unit case for dfToCols, which is dedicated for SparkR
now and its logic
Github user FRosner commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-151056747
@sun-rui I am not sure how callJStatic works but I can imagine that making
a method private does not allow it to be called - neither from the test, nor
from R. Why would
Github user sun-rui commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150803696
@FRosner, dfToCols is not a public API, and is now only a helper function
for SparkR. Could you add a private modifier for it?
---
If your project is set up for it, you
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150788661
Merged build finished. 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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150788665
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150788399
**[Test build #44293 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44293/consoleFull)**
for PR 9222 at commit
[`7e34d30`](https://git
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150778791
**[Test build #44293 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44293/consoleFull)**
for PR 9222 at commit
[`7e34d30`](https://gith
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150778341
Merged build started.
---
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
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150778337
Merged build triggered.
---
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 h
Github user FRosner commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150778223
@shivaram I added the missing license header. Sorry for missing this one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user FRosner commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150778118
@sun-rui thanks for your comments. You should not hesitate to add unit
tests for functions that are not tested, yet. Tests help other developers (like
me) to change stuf
Github user sun-rui commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150758802
@FRosner, it seems that you are investigating why the performance of the
old implementation does not behave as you expect? No matter the investigation
result, your new v
Github user sun-rui commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150757273
@FRosner, there is no test case dedicated for dfToCols in R now. I am still
reluctant to add a new test case in Scala simply for dfToCols. I'd like to
leave the decision
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700637
**[Test build #44262 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44262/consoleFull)**
for PR 9222 at commit
[`f857546`](https://git
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700643
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700641
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
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700586
**[Test build #44262 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44262/consoleFull)**
for PR 9222 at commit
[`f857546`](https://gith
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700188
Merged build triggered.
---
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 h
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150700209
Merged build started.
---
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
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150699863
Jenkins, 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 f
Github user Ge commented on the pull request:
https://github.com/apache/spark/pull/9222#issuecomment-150659258
The problem might be related to GC as a current implementation of
`dfToCols` allocates memory for the map result and the array while @FRosner
implementation only allocate
Github user FRosner commented on a diff in the pull request:
https://github.com/apache/spark/pull/9222#discussion_r42884105
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/api/r/SQLUtils.scala
---
@@ -130,16 +130,18 @@ private[r] object SQLUtils {
}
def d
28 matches
Mail list logo
