Skip to content

Conversation

@AngersZhuuuu
Copy link
Contributor

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 handle NaN like Null value

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

@github-actions github-actions bot added the SQL label Sep 10, 2021
@AngersZhuuuu
Copy link
Contributor Author

ping @cloud-fan

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

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

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

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

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

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

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

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

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

Test build #143136 has finished for PR 33951 at commit 5c515b8.

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

@SparkQA
Copy link

SparkQA commented Sep 10, 2021

Test build #143137 has finished for PR 33951 at commit cb928ff.

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

@srowen
Copy link
Member

srowen commented Sep 11, 2021

How does this differ from #33955 ?

@AngersZhuuuu
Copy link
Contributor Author

How does this differ from #33955 ?

#33955 make all flag in SQLOpenHashSet,

@HyukjinKwon
Copy link
Member

Mind elabourating #33951 (comment)? Also let's keep one PR only for the issue if we're clear on which approach to go ahead for.

@AngersZhuuuu AngersZhuuuu marked this pull request as draft September 12, 2021 02:03
@AngersZhuuuu
Copy link
Contributor Author

AngersZhuuuu commented Sep 12, 2021

Mind elabourating #33951 (comment)? Also let's keep one PR only for the issue if we're clear on which approach to go ahead for.

Close this, you can go ahead on that pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants