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/24 21:40:04 UTC
[GitHub] spark pull request #22542: [SPARK-25519][SQL] ArrayRemove function may retur...
GitHub user dilipbiswal opened a pull request:
https://github.com/apache/spark/pull/22542
[SPARK-25519][SQL] ArrayRemove function may return incorrect result when right expression is implicitly downcasted.
## What changes were proposed in this pull request?
In ArrayPosition, we currently cast the right hand side expression to match the element type of the left hand side Array. This may result in down casting and may return wrong result or questionable result.
Example :
```SQL
spark-sql> select array_remove(array(1,2,3), 1.23D);
[2,3]
```
```SQL
spark-sql> select array_remove(array(1,2,3), 'foo');
NULL
```
We should safely coerce both left and right hand side expressions.
## How was this patch tested?
Added tests in DataFrameFunctionsSuite
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dilipbiswal/spark SPARK-25519
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22542.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 #22542
----
commit ac9cb1a2d2e5b73f6c6e89c510f84a1f10efb60e
Author: Dilip Biswal <db...@...>
Date: 2018-09-16T02:36:18Z
[SPARK-25519] ArrayRemove function may return incorrect result when right expression is implicitly downcasted.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22542
@cloud-fan @ueshin I would like to ask a question here. There is one more function i wanted to fix. Originally i wanted to do it as part of this PR.. then realized that its not as straight forward and decided to do it in separate PR. The function in question is element_at. It turns out that we don't currently handle implicit casting between two Map types if the child types are different. say Map<int, string> to Map<long, string>. I was planning to enhance our implicitCast routine handle casting between MapTypes just like we do for ArrayType today. Does that sound okay to you ?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22542
LGTM. The PR description has a typo: `ArrayPosition` => `ArrayRemove`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22542
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96525/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22542
cc @ueshin @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22542
**[Test build #96525 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96525/testReport)** for PR 22542 at commit [`ac9cb1a`](https://github.com/apache/spark/commit/ac9cb1a2d2e5b73f6c6e89c510f84a1f10efb60e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/22542
thanks, merging to master/2.4!
@dilipbiswal sorry I didn't see your comment while merging. If the problem is about "implicit casting between two Map types", feel free to open a PR and let's see if we can backport it. thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22542: [SPARK-25519][SQL] ArrayRemove function may retur...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22542
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22542
**[Test build #96525 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96525/testReport)** for PR 22542 at commit [`ac9cb1a`](https://github.com/apache/spark/commit/ac9cb1a2d2e5b73f6c6e89c510f84a1f10efb60e).
* 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 #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22542
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 #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22542
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/3412/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22542
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 #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/22542
LGTM.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22542: [SPARK-25519][SQL] ArrayRemove function may return incor...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/22542
@cloud-fan Thanks a lot.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org