You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dilipbiswal <gi...@git.apache.org> on 2018/09/02 05:47:06 UTC
[GitHub] spark pull request #22315: [SPARK-25308][SQL] ArrayContains function may ret...
GitHub user dilipbiswal opened a pull request:
https://github.com/apache/spark/pull/22315
[SPARK-25308][SQL] ArrayContains function may return a error in the code generation phase.
## What changes were proposed in this pull request?
Invoking ArrayContains function with non nullable array type throws the following error in the code generation phase. Below is the error snippet.
```SQL
Code generation of array_contains([1,2,3], 1) failed:
java.util.concurrent.ExecutionException: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 40, Column 11: failed to compile: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 40, Column 11: Expression "isNull_0" is not an rvalue
java.util.concurrent.ExecutionException: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 40, Column 11: failed to compile: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 40, Column 11: Expression "isNull_0" is not an rvalue
at com.google.common.util.concurrent.AbstractFuture$Sync.getValue(AbstractFuture.java:306)
at com.google.common.util.concurrent.AbstractFuture$Sync.get(AbstractFuture.java:293)
at com.google.common.util.concurrent.AbstractFuture.get(AbstractFuture.java:116)
at com.google.common.util.concurrent.Uninterruptibles.getUninterruptibly(Uninterruptibles.java:135)
at com.google.common.cache.LocalCache$Segment.getAndRecordStats(LocalCache.java:2410)
at com.google.common.cache.LocalCache$Segment.loadSync(LocalCache.java:2380)
at com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache.java:2342)
at com.google.common.cache.LocalCache$Segment.get(LocalCache.java:2257)
at com.google.common.cache.LocalCache.get(LocalCache.java:4000)
at com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:4004)
at com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache.java:4874)
at org.apache.spark.sql.catalyst.expressions.codegen.CodeGenerator$.compile(CodeGenerator.scala:1305)
```
## How was this patch tested?
Added test in CollectionExpressionSuite.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dilipbiswal/spark SPARK-25308
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22315.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 #22315
----
commit 84b135c7176cd1affe3da449ede28adf182ae733
Author: Dilip Biswal <db...@...>
Date: 2018-08-31T02:59:30Z
[SPARK-25308] ArrayContains function may return a error in the code generation phase.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95638 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95638/testReport)** for PR 22315 at commit [`59ddb99`](https://github.com/apache/spark/commit/59ddb993790f4bb0ec920a2b0d897d8052c9f108).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95636/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22315
@ueshin Just verified in 2.3. This problem does not exist in 2.3. This is due to the fact that implementation of nullSafeCodeGen is different in 2.3 than in master. However, we are missing the test cases we added in these PRs in 2.3. Should we have the test cases checked in into the branch ? I am afraid that if we ever backported the pr that changed nullSafeCodeGen , we may introduce this bug. Please advise ..
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2805/
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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22315
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95636 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95636/testReport)** for PR 22315 at commit [`712542c`](https://github.com/apache/spark/commit/712542c0480bfb51317a6d4905bbaa0349c940e8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95636 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95636/testReport)** for PR 22315 at commit [`712542c`](https://github.com/apache/spark/commit/712542c0480bfb51317a6d4905bbaa0349c940e8).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214532578
--- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala ---
@@ -383,10 +383,13 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper
val a3 = Literal.create(null, ArrayType(StringType))
val a4 = Literal.create(Seq(create_row(1)), ArrayType(StructType(Seq(
StructField("a", IntegerType, true)))))
+ // Explicitly mark the array type not nullable (spark-xxxxx)
--- End diff --
nit: `spark-xxxxx`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2768/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95634 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95634/testReport)** for PR 22315 at commit [`712542c`](https://github.com/apache/spark/commit/712542c0480bfb51317a6d4905bbaa0349c940e8).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214542052
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,27 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
- }
+ val checkAndSetIsNullCode = if (nullable) {
--- End diff --
Could you update this part based on [this comment](https://github.com/apache/spark/pull/21103#discussion_r205999519) instead of `if (...) { ... } else`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22315
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95608 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95608/testReport)** for PR 22315 at commit [`c18fc08`](https://github.com/apache/spark/commit/c18fc08a5fce05bc947a558152e400e0c00806cc).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2803/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95589 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95589/testReport)** for PR 22315 at commit [`6f2881e`](https://github.com/apache/spark/commit/6f2881e8eb43611097d4a4d09313b20aa0b30233).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95593 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95593/testReport)** for PR 22315 at commit [`4190f42`](https://github.com/apache/spark/commit/4190f429ba3af22312d841eab5ae48d8886acede).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214564353
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,35 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
+ def checkAndSetIsNullCode(body: String) = {
+ if (nullable) {
+ s"""
+ |if ($arr.isNullAt($i)) {
--- End diff --
How about the case `right.nullable = true` and
`left.nullable = false AND left.dataType.asInstanceOf[ArrayType].containsNull = false`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95634 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95634/testReport)** for PR 22315 at commit [`712542c`](https://github.com/apache/spark/commit/712542c0480bfb51317a6d4905bbaa0349c940e8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22315
Thank you
LGTM except one comment.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95638/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2783/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2765/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95586/
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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
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/22315#discussion_r214647587
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,33 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
- }
+ def checkAndSetIsNullCode(body: String) = if (nullable) {
+ s"""
+ |if ($arr.isNullAt($i)) {
+ | ${ev.isNull} = true;
+ |} else {
+ | $body
+ |}
+ """.stripMargin
+ } else {
+ body
}
- """
+ val unsetIsNullCode = if (nullable) s"${ev.isNull} = false;" else ""
+ val code = checkAndSetIsNullCode(
--- End diff --
This seems too complicated to save a few duplicated code, how about
```
val loopBody = if (nullable) {
s"""
|if ($arr.isNullAt($i)) {
| ${ev.isNull} = true;
|} else ...
"""
} else {
s"""
|if (${ctx.genEqual(right.dataType, value, getValue)}) {...
"""
}
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2801/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95589 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95589/testReport)** for PR 22315 at commit [`6f2881e`](https://github.com/apache/spark/commit/6f2881e8eb43611097d4a4d09313b20aa0b30233).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/22315
@dilipbiswal Could we also add the test cases for the other high-order functions, if missing?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95608 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95608/testReport)** for PR 22315 at commit [`c18fc08`](https://github.com/apache/spark/commit/c18fc08a5fce05bc947a558152e400e0c00806cc).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95586/testReport)** for PR 22315 at commit [`84b135c`](https://github.com/apache/spark/commit/84b135c7176cd1affe3da449ede28adf182ae733).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95638 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95638/testReport)** for PR 22315 at commit [`59ddb99`](https://github.com/apache/spark/commit/59ddb993790f4bb0ec920a2b0d897d8052c9f108).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95587 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95587/testReport)** for PR 22315 at commit [`84b135c`](https://github.com/apache/spark/commit/84b135c7176cd1affe3da449ede28adf182ae733).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22315
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22315
@dilipbiswal Do we need to backport this to 2.3? If so, could you submit a backport pr to branch-2.3 please? Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95589/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22315
@gatorsmile Sure.. I will check and add.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by mgaido91 <gi...@git.apache.org>.
Github user mgaido91 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214532569
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,28 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
- }
+ val checkAndSetIsNullCode = if (nullable) {
+ s"""
+ |if ($arr.isNullAt($i)) {
+ | ${ev.isNull} = true;
+ |} else
+ |
--- End diff --
nit: extra newline
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2770/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22315
LGTM.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214596495
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,35 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
+ def checkAndSetIsNullCode(body: String) = {
--- End diff --
@maropu ok..
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95586 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95586/testReport)** for PR 22315 at commit [`84b135c`](https://github.com/apache/spark/commit/84b135c7176cd1affe3da449ede28adf182ae733).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95634/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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/2764/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22315
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95593/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95587 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95587/testReport)** for PR 22315 at commit [`84b135c`](https://github.com/apache/spark/commit/84b135c7176cd1affe3da449ede28adf182ae733).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22315
@dilipbiswal Thanks for the verification! I don't think we will backport the `nullSafeCodeGen` to branch-2.3, but feel free to submit a pr to backport the test cases to branch-2.3.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214596427
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,35 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
+ def checkAndSetIsNullCode(body: String) = {
+ if (nullable) {
+ s"""
+ |if ($arr.isNullAt($i)) {
--- End diff --
@maropu This should be rejected in checkInputDataTypes(), no ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95587/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22315
Thanks! merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95608/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
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 #22315: [SPARK-25308][SQL] ArrayContains function may ret...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/22315#discussion_r214564677
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala ---
@@ -1464,17 +1464,35 @@ case class ArrayContains(left: Expression, right: Expression)
nullSafeCodeGen(ctx, ev, (arr, value) => {
val i = ctx.freshName("i")
val getValue = CodeGenerator.getValue(arr, right.dataType, i)
- s"""
- for (int $i = 0; $i < $arr.numElements(); $i ++) {
- if ($arr.isNullAt($i)) {
- ${ev.isNull} = true;
- } else if (${ctx.genEqual(right.dataType, value, getValue)}) {
- ${ev.isNull} = false;
- ${ev.value} = true;
- break;
+ def checkAndSetIsNullCode(body: String) = {
--- End diff --
nit: How about ` def checkAndSetIsNullCode(body: String) = if (nullable) {`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22315
**[Test build #95593 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95593/testReport)** for PR 22315 at commit [`4190f42`](https://github.com/apache/spark/commit/4190f429ba3af22312d841eab5ae48d8886acede).
* 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 #22315: [SPARK-25308][SQL] ArrayContains function may return a e...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22315
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org