[SPARK-36741][SQL] ArrayDistinct handle duplicated Double.NaN and Float.Nan by AngersZhuuuu · Pull Request #33993 · apache/spark

AngersZhuuuu · 2021-09-14T13:26:03Z

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

…at.Nan

SparkQA · 2021-09-14T14:28:56Z

Kubernetes integration test unable to build dist.

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

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala

SparkQA · 2021-09-14T17:32:42Z

Kubernetes integration test unable to build dist.

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

SparkQA · 2021-09-14T18:41:20Z

Test build #143262 has finished for PR 33993 at commit 0ac9924.

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

dongjoon-hyun · 2021-09-14T19:31:38Z

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

+          val elem = array.get(i, elementType)
+          if (isNaN(elem)) {
+            if (!hs.containsNaN) {
+              arrayBuffer += elem


For this, let's wait for the decision at the first PR.

https://github.com/apache/spark/pull/33955/files#r708570515

For this, let's wait for the decision at the first PR.

https://github.com/apache/spark/pull/33955/files#r708570515

Done

SparkQA · 2021-09-14T21:44:16Z

Test build #143267 has finished for PR 33993 at commit 63763df.

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

AngersZhuuuu · 2021-09-15T14:26:48Z

ping @cloud-fan @dongjoon-hyun

cloud-fan · 2021-09-15T14:30:23Z

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

          }

-        val processArray = withArrayNullAssignment(
+        def withNaNCheck(body: String): String = {


shall we move these codegen utils to SQLOpenHashSet as well to reduce duplicated code?

shall we move these codegen utils to SQLOpenHashSet as well to reduce duplicated code?

How about to do this after all done. Not only this can be refactored.

@cloud-fan Updated, How about current?

SparkQA · 2021-09-15T15:47:13Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47809/

SparkQA · 2021-09-15T15:57:28Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47809/

SparkQA · 2021-09-15T19:54:31Z

Test build #143307 has finished for PR 33993 at commit f5c5452.

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

SparkQA · 2021-09-16T09:04:43Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47848/

SparkQA · 2021-09-16T09:15:22Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47848/

cloud-fan · 2021-09-16T09:21:22Z

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

  }

-  def valueNaN(dataType: DataType): Any = {
+  def isNaNFuncAndValueNaN(dataType: DataType, valueName: String): Option[(String, String)] = {


seems we can remove it.

SparkQA · 2021-09-16T10:05:22Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47850/

SparkQA · 2021-09-16T10:14:30Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47850/

SparkQA · 2021-09-16T10:16:15Z

Test build #143345 has finished for PR 33993 at commit 202cf4e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-09-16T10:42:25Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47851/

SparkQA · 2021-09-16T18:06:14Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47867/

SparkQA · 2021-09-16T21:53:48Z

Test build #143360 has finished for PR 33993 at commit 389c9fd.

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

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

SparkQA · 2021-09-17T04:04:53Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47892/

SparkQA · 2021-09-17T04:13:18Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47894/

SparkQA · 2021-09-17T04:14:01Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47892/

SparkQA · 2021-09-17T04:23:08Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47894/

SparkQA · 2021-09-17T04:45:24Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47895/

SparkQA · 2021-09-17T04:55:19Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47895/

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

SparkQA · 2021-09-17T06:19:08Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47902/

SparkQA · 2021-09-17T06:29:08Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47902/

SparkQA · 2021-09-17T07:53:15Z

Test build #143385 has finished for PR 33993 at commit d8e80fc.

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

SparkQA · 2021-09-17T08:54:08Z

Test build #143387 has finished for PR 33993 at commit 21e9422.

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

SparkQA · 2021-09-17T10:00:25Z

Test build #143391 has finished for PR 33993 at commit 434a2be.

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

AngersZhuuuu · 2021-09-17T10:07:09Z

ping @cloud-fan

…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>

cloud-fan · 2021-09-17T12:53:27Z

thanks, merging to master/3.2/3.1/3.0!

…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>

AngersZhuuuu added 2 commits September 14, 2021 21:24

[SPARK-36742][SQL] ArrayDistinct handle duplicated Double.NaN and Flo…

1b39da5

…at.Nan

Update collectionOperations.scala

0ac9924

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

AngersZhuuuu changed the title ~~[SPARK-36742][SQL] ArrayDistinct handle duplicated Double.NaN and Float.Nan~~ [SPARK-36741][SQL] ArrayDistinct handle duplicated Double.NaN and Float.Nan Sep 14, 2021

dongjoon-hyun reviewed Sep 14, 2021

View reviewed changes

...st/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala Outdated Show resolved Hide resolved

Update CollectionExpressionsSuite.scala

63763df

dongjoon-hyun reviewed Sep 14, 2021

View reviewed changes

AngersZhuuuu added 2 commits September 15, 2021 22:22

Merge branch 'master' into SPARK-36741

55f230d

Update collectionOperations.scala

7bc972f

cloud-fan reviewed Sep 15, 2021

View reviewed changes

Merge branch 'master' into SPARK-36741

f5c5452

refactor

2478eb4

follow comment

202cf4e

cloud-fan reviewed Sep 16, 2021

View reviewed changes

Update SQLOpenHashSet.scala

5546a55

update

72870f6

Update collectionOperations.scala

d8e80fc

cloud-fan reviewed Sep 17, 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 17, 2021 11:15

update

d0a914c

Update SQLOpenHashSet.scala

6194de6

Update collectionOperations.scala

21e9422

cloud-fan reviewed Sep 17, 2021

View reviewed changes

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

cloud-fan approved these changes Sep 17, 2021

View reviewed changes

Update collectionOperations.scala

434a2be

cloud-fan closed this in e356f6a Sep 17, 2021

nchammas mentioned this pull request Feb 12, 2024

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

Closed

revans2 mentioned this pull request Apr 10, 2024

[FEA] Support function array_distinct NVIDIA/spark-rapids#5221

Closed

Conversation

AngersZhuuuu commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

SparkQA commented Sep 14, 2021

Uh oh!

Uh oh!

SparkQA commented Sep 14, 2021

Uh oh!

SparkQA commented Sep 14, 2021

Uh oh!

dongjoon-hyun Sep 14, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 15, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 14, 2021

Uh oh!

AngersZhuuuu commented Sep 15, 2021

Uh oh!

cloud-fan Sep 15, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 15, 2021

Uh oh!

SparkQA commented Sep 15, 2021

Uh oh!

SparkQA commented Sep 15, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

cloud-fan Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

AngersZhuuuu Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

SparkQA commented Sep 16, 2021

Uh oh!

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

Uh oh!

SparkQA commented Sep 17, 2021

Uh oh!

SparkQA commented Sep 17, 2021

AngersZhuuuu commented Sep 14, 2021 •

edited

Loading

AngersZhuuuu Sep 15, 2021 •

edited

Loading