You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sadhen <gi...@git.apache.org> on 2018/08/29 03:22:28 UTC
[GitHub] spark pull request #22260: [MINOR] Fix scala 2.12 build using collect
GitHub user sadhen opened a pull request:
https://github.com/apache/spark/pull/22260
[MINOR] Fix scala 2.12 build using collect
## What changes were proposed in this pull request?
Introduced by #21320
```
[error] [warn] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala:41: match may not be exhaustive.
[error] It would fail on the following inputs: (_, ArrayType(_, _)), (_, _)
[error] [warn] getProjection(a.child).map(p => (p, p.dataType)).map {
[error] [warn]
[error] [warn] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala:52: match may not be exhaustive.
[error] It would fail on the following input: (_, _)
[error] [warn] getProjection(child).map(p => (p, p.dataType)).map {
[error] [warn]
```
```
$ sbt
> ++2.12.6
> project sql
> compile
```
## How was this patch tested?
Existing tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sadhen/spark fix_exhaustive_match
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22260.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 #22260
----
commit 626f26598ccfc692cca59e29a2d7861133654ef0
Author: 忍冬 <re...@...>
Date: 2018-08-29T03:14:11Z
Fix exhaustive match using collect
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 #22260: [MINOR] Fix scala 2.12 build using collect
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 pull request #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213884559
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSparkSubmitSuite.scala ---
@@ -19,7 +19,7 @@ package org.apache.spark.sql.hive
import java.io.{BufferedWriter, File, FileWriter}
-import scala.tools.nsc.Properties
+import scala.util.Properties
--- End diff --
`scala.tools` is part of scala-compiler
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213573236
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala ---
@@ -38,7 +38,7 @@ private[execution] case class ProjectionOverSchema(schema: StructType) {
case GetArrayItem(child, arrayItemOrdinal) =>
getProjection(child).map { projection => GetArrayItem(projection, arrayItemOrdinal) }
case a: GetArrayStructFields =>
- getProjection(a.child).map(p => (p, p.dataType)).map {
+ getProjection(a.child).map(p => (p, p.dataType)).collect {
--- End diff --
Well, there is a semantic change using `collect`. I will provide a unit test to verify the change or simply using `sys.error`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213884979
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala ---
@@ -38,7 +38,7 @@ private[execution] case class ProjectionOverSchema(schema: StructType) {
case GetArrayItem(child, arrayItemOrdinal) =>
getProjection(child).map { projection => GetArrayItem(projection, arrayItemOrdinal) }
case a: GetArrayStructFields =>
- getProjection(a.child).map(p => (p, p.dataType)).map {
+ getProjection(a.child).map(p => (p, p.dataType)).collect {
--- End diff --
To use `collect`, one must dive into `SPARK-4502`.
This PR is intended for #22264 .
I will preserve the error behavior.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95452 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95452/testReport)** for PR 22260 at commit [`2dff17b`](https://github.com/apache/spark/commit/2dff17b8288a8c3cd82b2d3514bfc2d1f06d88d5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213884075
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSparkSubmitSuite.scala ---
@@ -19,7 +19,7 @@ package org.apache.spark.sql.hive
import java.io.{BufferedWriter, File, FileWriter}
-import scala.tools.nsc.Properties
+import scala.util.Properties
--- End diff --
Yes. And
`scala.util.Properties` is better than `scala.tools.nsc.Properties` !
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95452 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95452/testReport)** for PR 22260 at commit [`2dff17b`](https://github.com/apache/spark/commit/2dff17b8288a8c3cd82b2d3514bfc2d1f06d88d5).
* 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 #22260: [MINOR] Fix scala 2.12 build using collect
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22260
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 #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95406 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95406/testReport)** for PR 22260 at commit [`ffdb12d`](https://github.com/apache/spark/commit/ffdb12d4ffafd9ae0f48469e2f4db9e27444e724).
* 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 #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213764969
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveSparkSubmitSuite.scala ---
@@ -19,7 +19,7 @@ package org.apache.spark.sql.hive
import java.io.{BufferedWriter, File, FileWriter}
-import scala.tools.nsc.Properties
+import scala.util.Properties
--- End diff --
This works in 2.11 and 2.12?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [MINOR] Fix scala 2.12 build using collect
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95401/testReport)** for PR 22260 at commit [`626f265`](https://github.com/apache/spark/commit/626f26598ccfc692cca59e29a2d7861133654ef0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/22260
LGTM, too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [MINOR] Fix scala 2.12 build using collect
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 #22260: [WIP][SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95401/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [MINOR] Fix scala 2.12 build using collect
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 pull request #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213767758
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala ---
@@ -38,7 +38,7 @@ private[execution] case class ProjectionOverSchema(schema: StructType) {
case GetArrayItem(child, arrayItemOrdinal) =>
getProjection(child).map { projection => GetArrayItem(projection, arrayItemOrdinal) }
case a: GetArrayStructFields =>
- getProjection(a.child).map(p => (p, p.dataType)).map {
+ getProjection(a.child).map(p => (p, p.dataType)).collect {
--- End diff --
Does this generate an error or just a warning in 2.12? It's an unmatched case error, right?
We had 'fixed' this elsewhere with `case _ => throw new IllegalStateException(...)` or something. At runtime it would have caused a MatchError before, right? Because it didn't, I hope, or else tests would have failed, it matters a bit less what happens in this case. But it felt more conservative to preserve the error behavior; maybe I was wrong about that?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [MINOR] Fix scala 2.12 build using collect
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/22260
Can you add `[SQL][MINOR]` in the title? Also, can you narrow down the tittle cuz it is a little obscure.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95406 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95406/testReport)** for PR 22260 at commit [`ffdb12d`](https://github.com/apache/spark/commit/ffdb12d4ffafd9ae0f48469e2f4db9e27444e724).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95452/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95406/
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 #22260: [MINOR] Fix scala 2.12 build using collect
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/22260#discussion_r213536395
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ProjectionOverSchema.scala ---
@@ -38,7 +38,7 @@ private[execution] case class ProjectionOverSchema(schema: StructType) {
case GetArrayItem(child, arrayItemOrdinal) =>
getProjection(child).map { projection => GetArrayItem(projection, arrayItemOrdinal) }
case a: GetArrayStructFields =>
- getProjection(a.child).map(p => (p, p.dataType)).map {
+ getProjection(a.child).map(p => (p, p.dataType)).collect {
--- End diff --
How about this? IMO `.collect` can't catch illegal inputs?
```
getProjection(a.child).map(p => (p, p.dataType)).map {
case (projection, ArrayType(projSchema @ StructType(_), _)) =>
GetArrayStructFields(projection,
projSchema(a.field.name),
projSchema.fieldIndex(a.field.name),
projSchema.size,
a.containsNull)
case _ =>
sys.error("....")
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22260
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [WIP][SQL][MINOR] Fix compiling for scala 2.12
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22260
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 #22260: [WIP][SQL][MINOR] Fix compiling for scala 2.12
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22260
**[Test build #95401 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95401/testReport)** for PR 22260 at commit [`626f265`](https://github.com/apache/spark/commit/626f26598ccfc692cca59e29a2d7861133654ef0).
* This patch **fails due to an unknown error code, -9**.
* 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 #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22260
Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22260: [SQL][MINOR] Fix compiling for scala 2.12
Posted by sadhen <gi...@git.apache.org>.
Github user sadhen commented on the issue:
https://github.com/apache/spark/pull/22260
@maropu @srowen please review
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org