[SPARK-36702][SQL] ArrayUnion handle duplicated Double.NaN and Float.Nan by AngersZhuuuu · Pull Request #33955 · apache/spark

AngersZhuuuu · 2021-09-10T09:01:09Z

What changes were proposed in this pull request?

For query

select array_union(array(cast('nan' as double), cast('nan' as double)), array())

This returns [NaN, NaN], but it should return [NaN].
This issue is caused by OpenHashSet can't handle Double.NaN and Float.NaN too.
In this pr we add a wrap for OpenHashSet that can handle null, Double.NaN, Float.NaN together

Why are the changes needed?

Fix bug

Does this PR introduce any user-facing change?

ArrayUnion won't show duplicated NaN value

How was this patch tested?

Added UT

AngersZhuuuu · 2021-09-10T09:13:35Z

ping @cloud-fan

SparkQA · 2021-09-10T10:13:51Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47646/

sql/catalyst/src/main/scala/org/apache/spark/sql/util/SQLOpenHashSet.scala

cloud-fan · 2021-09-10T12:51:09Z

sql/catalyst/src/main/scala/org/apache/spark/sql/util/SQLOpenHashSet.scala

+
+  private var containsNull = false
+  private var containsDoubleNaN = false
+  private var containsFloatNaN = false


The data added to this set will always be the same data type. I think we can just have a single containsNaN flag.

The data added to this set will always be the same data type. I think we can just have a single containsNaN flag.

I have thought about this too, but since it can support any type, so keep this may be better?

Maybe we should do the null/nan check at the caller side

class SQLOpenHashSet ... { def add(k: T) def addNull() def addNaN() } // caller side if (row.isNullAt...) { set.addNull() } else { ... if (java.lang.Double.isNaN(value)) { set.addNaN() } }

How about current

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

SparkQA · 2021-09-10T16:12:14Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47656/

sql/catalyst/src/main/scala/org/apache/spark/sql/util/SQLOpenHashSet.scala

SparkQA · 2021-09-10T17:23:42Z

Test build #143142 has finished for PR 33955 at commit 8579c97.

This patch fails from timeout after a configured wait of 500m.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class SQLOpenHashSet[@specialized(Long, Int, Double, Float) T: ClassTag](

SparkQA · 2021-09-10T20:25:09Z

Test build #143152 has finished for PR 33955 at commit 1857988.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-11T14:17:24Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47669/

SparkQA · 2021-09-11T18:24:07Z

Test build #143165 has finished for PR 33955 at commit 4e533fd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AngersZhuuuu · 2021-09-13T02:36:58Z

ping @cloud-fan

SparkQA · 2021-09-13T02:47:16Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47685/

SparkQA · 2021-09-13T02:49:57Z

Test build #143182 has finished for PR 33955 at commit 119679c.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-13T03:46:51Z

Test build #143186 has finished for PR 33955 at commit 45d1fee.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-13T03:50:02Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47688/

…rsZhuuuu/spark into SPARK-36702-WrapOpenHashSet

SparkQA · 2021-09-13T05:20:54Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47690/

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

AngersZhuuuu · 2021-09-15T15:01:48Z

@HyukjinKwon This commit pass the check ac8bce8 https://github.com/apache/spark/pull/34006/commits

cloud-fan · 2021-09-15T15:13:54Z

I fetched the latest master and the test passed on my side.

HyukjinKwon · 2021-09-16T00:04:06Z

Thanks guys.This is possibly flaky. I'll keep my eyes on the build.

HyukjinKwon · 2021-09-17T05:51:09Z

I still see this test failure, see https://github.com/apache/spark/runs/3628995384. Shall we revert this PR?

HyukjinKwon · 2021-09-17T05:54:31Z

Actually there are some more: https://github.com/apache/spark/runs/3619357249

cloud-fan · 2021-09-17T06:05:55Z

This is so weird. There is no randomness in the test. How frequently do we see the test failure?

…lder in array functions ### What changes were proposed in this pull request? In array functions, we use constant 0 as the placeholder when adding a null value to an array buffer. This PR makes sure the constant 0 matches the type of the array element. ### Why are the changes needed? Fix a potential bug. Somehow we can hit this bug sometimes after #33955 . ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes #34029 from cloud-fan/minor. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…lder in array functions ### What changes were proposed in this pull request? In array functions, we use constant 0 as the placeholder when adding a null value to an array buffer. This PR makes sure the constant 0 matches the type of the array element. ### Why are the changes needed? Fix a potential bug. Somehow we can hit this bug sometimes after #33955 . ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes #34029 from cloud-fan/minor. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 4145498) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…lder in array functions In array functions, we use constant 0 as the placeholder when adding a null value to an array buffer. This PR makes sure the constant 0 matches the type of the array element. Fix a potential bug. Somehow we can hit this bug sometimes after #33955 . No existing tests Closes #34029 from cloud-fan/minor. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 4145498) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…at.Nan ### What changes were proposed in this pull request? For query ``` select array_distinct(array(cast('nan' as double), cast('nan' as double))) ``` This returns [NaN, NaN], but it should return [NaN]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayDistinct won't show duplicated `NaN` value ### How was this patch tested? Added UT Closes #33993 from AngersZhuuuu/SPARK-36741. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…at.Nan ### What changes were proposed in this pull request? For query ``` select array_distinct(array(cast('nan' as double), cast('nan' as double))) ``` This returns [NaN, NaN], but it should return [NaN]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayDistinct won't show duplicated `NaN` value ### How was this patch tested? Added UT Closes #33993 from AngersZhuuuu/SPARK-36741. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit e356f6a) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…at.Nan For query ``` select array_distinct(array(cast('nan' as double), cast('nan' as double))) ``` This returns [NaN, NaN], but it should return [NaN]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 Fix bug ArrayDistinct won't show duplicated `NaN` value Added UT Closes #33993 from AngersZhuuuu/SPARK-36741. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit e356f6a) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…oat.NaN ### What changes were proposed in this pull request? For query ``` select array_intersect(array(cast('nan' as double), 1d), array(cast('nan' as double))) ``` This returns [NaN], but it should return []. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayIntersect won't show equal `NaN` value ### How was this patch tested? Added UT Closes #33995 from AngersZhuuuu/SPARK-36754. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…oat.NaN ### What changes were proposed in this pull request? For query ``` select array_intersect(array(cast('nan' as double), 1d), array(cast('nan' as double))) ``` This returns [NaN], but it should return []. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayIntersect won't show equal `NaN` value ### How was this patch tested? Added UT Closes #33995 from AngersZhuuuu/SPARK-36754. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 2fc7f2f) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

….NaN ### What changes were proposed in this pull request? For query ``` select array_except(array(cast('nan' as double), 1d), array(cast('nan' as double))) ``` This returns [NaN, 1d], but it should return [1d]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayExcept won't show handle equal `NaN` value ### How was this patch tested? Added UT Closes #33994 from AngersZhuuuu/SPARK-36753. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

….NaN ### What changes were proposed in this pull request? For query ``` select array_except(array(cast('nan' as double), 1d), array(cast('nan' as double))) ``` This returns [NaN, 1d], but it should return [1d]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on #33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayExcept won't show handle equal `NaN` value ### How was this patch tested? Added UT Closes #33994 from AngersZhuuuu/SPARK-36753. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit a7cbe69) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

….NaN ### What changes were proposed in this pull request? For query ``` select array_except(array(cast('nan' as double), 1d), array(cast('nan' as double))) ``` This returns [NaN, 1d], but it should return [1d]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on apache/spark#33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayExcept won't show handle equal `NaN` value ### How was this patch tested? Added UT Closes #33994 from AngersZhuuuu/SPARK-36753. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

### What changes were proposed in this pull request? For query ``` select array_union(array(cast('nan' as double), cast('nan' as double)), array()) ``` This returns [NaN, NaN], but it should return [NaN]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr we add a wrap for OpenHashSet that can handle `null`, `Double.NaN`, `Float.NaN` together ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayUnion won't show duplicated `NaN` value ### How was this patch tested? Added UT Closes apache#33955 from AngersZhuuuu/SPARK-36702-WrapOpenHashSet. Lead-authored-by: Angerszhuuuu <angers.zhu@gmail.com> Co-authored-by: AngersZhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit f71f377) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…and Float.NaN ### What changes were proposed in this pull request? According to apache#33955 (comment) use normalized NaN ### Why are the changes needed? Use normalized NaN for duplicated NaN value ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Exiting UT Closes apache#34003 from AngersZhuuuu/SPARK-36702-FOLLOWUP. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 6380859) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…lder in array functions ### What changes were proposed in this pull request? In array functions, we use constant 0 as the placeholder when adding a null value to an array buffer. This PR makes sure the constant 0 matches the type of the array element. ### Why are the changes needed? Fix a potential bug. Somehow we can hit this bug sometimes after apache#33955 . ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests Closes apache#34029 from cloud-fan/minor. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 4145498) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…at.Nan ### What changes were proposed in this pull request? For query ``` select array_distinct(array(cast('nan' as double), cast('nan' as double))) ``` This returns [NaN, NaN], but it should return [NaN]. This issue is caused by `OpenHashSet` can't handle `Double.NaN` and `Float.NaN` too. In this pr fix this based on apache#33955 ### Why are the changes needed? Fix bug ### Does this PR introduce _any_ user-facing change? ArrayDistinct won't show duplicated `NaN` value ### How was this patch tested? Added UT Closes apache#33993 from AngersZhuuuu/SPARK-36741. Authored-by: Angerszhuuuu <angers.zhu@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit e356f6a) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

[SPARK-36702][SQL] ArrayUnion handle duplicated Double.NaN and Float.Nan

8579c97

github-actions bot added the SQL label Sep 10, 2021

cloud-fan reviewed Sep 10, 2021

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/util/SQLOpenHashSet.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Sep 10, 2021

View reviewed changes

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated Show resolved Hide resolved

AngersZhuuuu added 3 commits September 10, 2021 23:26

follow comment

d6e4f39

Update collectionOperations.scala

a481196

Update collectionOperations.scala

1857988

cfmcgrady reviewed Sep 10, 2021

View reviewed changes

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated Show resolved Hide resolved

cfmcgrady reviewed Sep 10, 2021

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/util/SQLOpenHashSet.scala Outdated Show resolved Hide resolved

Update collectionOperations.scala

4e533fd

srowen mentioned this pull request Sep 11, 2021

[SPARK-36702][SQL] ArrayUnion handle duplicated Double.NaN and Float.Nan #33951

Closed

update

119679c

Update SQLOpenHashSet.scala

45d1fee

AngersZhuuuu added 2 commits September 13, 2021 11:59

update

08cc967

Merge branch 'SPARK-36702-WrapOpenHashSet' of https://github.com/Ange…

8d0e4a9

…rsZhuuuu/spark into SPARK-36702-WrapOpenHashSet

cloud-fan reviewed Sep 13, 2021

View reviewed changes

...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala Outdated Show resolved Hide resolved

cloud-fan mentioned this pull request Sep 17, 2021

[SPARK-36789][SQL] Use the correct constant type as the null value holder in array functions #34029

Closed

NVnavkumar mentioned this pull request Jul 23, 2022

Add set based operations for arrays: array_intersect, array_union, array_except, and arrays_overlap for running on GPU NVIDIA/spark-rapids#5958

Merged

nchammas mentioned this pull request Feb 12, 2024

[SPARK-45599][CORE] Use object equality in OpenHashSet #45036

Closed

Conversation

AngersZhuuuu commented Sep 10, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

AngersZhuuuu commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

Uh oh!

cloud-fan Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

cloud-fan Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 10, 2021

Uh oh!

SparkQA commented Sep 11, 2021

Uh oh!

SparkQA commented Sep 11, 2021

Uh oh!

AngersZhuuuu commented Sep 13, 2021

Uh oh!

SparkQA commented Sep 13, 2021

Uh oh!

SparkQA commented Sep 13, 2021

Uh oh!

SparkQA commented Sep 13, 2021

Uh oh!

SparkQA commented Sep 13, 2021

Uh oh!

SparkQA commented Sep 13, 2021

Uh oh!

Uh oh!

AngersZhuuuu commented Sep 15, 2021

Uh oh!

cloud-fan commented Sep 15, 2021

Uh oh!

HyukjinKwon commented Sep 16, 2021

Uh oh!

HyukjinKwon commented Sep 17, 2021

Uh oh!

HyukjinKwon commented Sep 17, 2021

Uh oh!

cloud-fan commented Sep 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants