You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2015/12/14 02:43:12 UTC
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/10285
[SPARK-12288] [SQL] Support UnsafeRow in Coalesce/Except/Intersect.
Support UnsafeRow for the Coalesce/Except/Intersect.
Could you review if my code changes are ok? @davies Thank you!
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark unsafeSupportCIE
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10285.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 #10285
----
commit cbb2c597fafff78c9207b733197f6391421bf591
Author: gatorsmile <ga...@gmail.com>
Date: 2015-12-14T01:37:33Z
Support UnsafeRow in Coalesce/Except/Intersect.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47588380
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,41 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
--- End diff --
Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164609125
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47687/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164539541
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47661/
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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47464659
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,48 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
+
+ test("coalesce can process safe rows") {
--- End diff --
We may remove the safe rows in future, it's fine to not have these tests (for safe rows).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164628549
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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47585423
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,48 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
+
+ test("coalesce can process safe rows") {
--- End diff --
Sorry, doing it now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164612191
**[Test build #47698 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47698/consoleFull)** for PR 10285 at commit [`b3b70af`](https://github.com/apache/spark/commit/b3b70af7fce615454d5bee1f492ce84638f6b045).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164572517
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47674/
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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164547703
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164550825
**[Test build #47674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47674/consoleFull)** for PR 10285 at commit [`b17ba61`](https://github.com/apache/spark/commit/b17ba61a4e6873ed93d6cb9115b58ee78581726a).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164539528
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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164572513
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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164608995
**[Test build #47687 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47687/consoleFull)** for PR 10285 at commit [`b17ba61`](https://github.com/apache/spark/commit/b17ba61a4e6873ed93d6cb9115b58ee78581726a).
* This patch passes all 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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47588279
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,41 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
--- End diff --
oh sorry, I thought it was `org.apache.spark.sql.catalyst.expressions.Coalesce`, nvm
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47542996
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,48 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
+
+ test("coalesce can process safe rows") {
--- End diff --
uh,,, I see! : )
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47465230
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala ---
@@ -263,6 +265,10 @@ case class Except(left: SparkPlan, right: SparkPlan) extends BinaryNode {
protected override def doExecute(): RDD[InternalRow] = {
left.execute().map(_.copy()).subtract(right.execute().map(_.copy()))
}
+
+ override def outputsUnsafeRows: Boolean = children.forall(_.outputsUnsafeRows)
+ override def canProcessUnsafeRows: Boolean = true
--- End diff --
seems it's already promised in https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/rowFormatConverters.scala#L95-L106
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164628471
**[Test build #47698 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47698/consoleFull)** for PR 10285 at commit [`b3b70af`](https://github.com/apache/spark/commit/b3b70af7fce615454d5bee1f492ce84638f6b045).
* This patch passes all 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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164594077
**[Test build #47687 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47687/consoleFull)** for PR 10285 at commit [`b17ba61`](https://github.com/apache/spark/commit/b17ba61a4e6873ed93d6cb9115b58ee78581726a).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164579754
The failed test case is not related to my change. I will try to see if I can fix the flaky test.
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164333105
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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164572436
**[Test build #47674 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47674/consoleFull)** for PR 10285 at commit [`b17ba61`](https://github.com/apache/spark/commit/b17ba61a4e6873ed93d6cb9115b58ee78581726a).
* This patch **fails SparkR 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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47542952
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala ---
@@ -263,6 +265,10 @@ case class Except(left: SparkPlan, right: SparkPlan) extends BinaryNode {
protected override def doExecute(): RDD[InternalRow] = {
left.execute().map(_.copy()).subtract(right.execute().map(_.copy()))
}
+
+ override def outputsUnsafeRows: Boolean = children.forall(_.outputsUnsafeRows)
+ override def canProcessUnsafeRows: Boolean = true
--- End diff --
Thank you! @davies @cloud-fan I added two extra test cases for ensuring they have the same formats.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164592993
retest this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164321716
**[Test build #47633 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47633/consoleFull)** for PR 10285 at commit [`cbb2c59`](https://github.com/apache/spark/commit/cbb2c597fafff78c9207b733197f6391421bf591).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47587750
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,41 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
--- End diff --
Sorry, I am not sure if I understand your question. `Coalesce` only have a single child. I am not sure what mixed rows mean. Could you give me one example? Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10285
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47586476
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,41 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
--- End diff --
should we add mixed case test for coalesce too?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47580612
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/RowFormatConvertersSuite.scala ---
@@ -58,6 +58,48 @@ class RowFormatConvertersSuite extends SparkPlanTest with SharedSQLContext {
assert(!preparedPlan.outputsUnsafeRows)
}
+ test("coalesce can process unsafe rows") {
+ val plan = Coalesce(1, outputsUnsafe)
+ val preparedPlan = sqlContext.prepareForExecution.execute(plan)
+ assert(getConverters(preparedPlan).size === 1)
+ assert(preparedPlan.outputsUnsafeRows)
+ }
+
+ test("coalesce can process safe rows") {
--- End diff --
Will you remove these?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164333027
**[Test build #47633 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47633/consoleFull)** for PR 10285 at commit [`cbb2c59`](https://github.com/apache/spark/commit/cbb2c597fafff78c9207b733197f6391421bf591).
* This patch passes all 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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164636137
LGTM, merging into master, 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 pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164609124
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 does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47464584
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala ---
@@ -263,6 +265,10 @@ case class Except(left: SparkPlan, right: SparkPlan) extends BinaryNode {
protected override def doExecute(): RDD[InternalRow] = {
left.execute().map(_.copy()).subtract(right.execute().map(_.copy()))
}
+
+ override def outputsUnsafeRows: Boolean = children.forall(_.outputsUnsafeRows)
+ override def canProcessUnsafeRows: Boolean = true
--- End diff --
UnsafeRow can be compared with GenericInternalRow, so we should make sure that `left` and `right` have the same type of row.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47542822
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala ---
@@ -263,6 +265,10 @@ case class Except(left: SparkPlan, right: SparkPlan) extends BinaryNode {
protected override def doExecute(): RDD[InternalRow] = {
left.execute().map(_.copy()).subtract(right.execute().map(_.copy()))
}
+
+ override def outputsUnsafeRows: Boolean = children.forall(_.outputsUnsafeRows)
--- End diff --
Thank you! I also found the same issue exists in ```Union``` I also did a correction.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164628551
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47698/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/10285#discussion_r47464475
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/basicOperators.scala ---
@@ -263,6 +265,10 @@ case class Except(left: SparkPlan, right: SparkPlan) extends BinaryNode {
protected override def doExecute(): RDD[InternalRow] = {
left.execute().map(_.copy()).subtract(right.execute().map(_.copy()))
}
+
+ override def outputsUnsafeRows: Boolean = children.forall(_.outputsUnsafeRows)
--- End diff --
If an operator's children produce both unsafe and safe rows, we will convert everything unsafe rows.
so here should be `children.exists(_.outputsUnsafeRows)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12288] [SQL] Support UnsafeRow in Coale...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10285#issuecomment-164333106
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47633/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org