Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix array equal check #571

Merged
merged 3 commits into from
Jul 20, 2021
Merged

Fix array equal check #571

merged 3 commits into from
Jul 20, 2021

Conversation

waynexia
Copy link
Member

Which issue does this PR close?

Closes #570.

What changes are included in this PR?

This PR fixes the problem mentioned in #570 and adds some negative test cases.

Are there any user-facing changes?

No.

waynexia added 3 commits July 20, 2021 16:37
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
Signed-off-by: Ruihang Xia <[email protected]>
@github-actions github-actions bot added the arrow Changes to the arrow crate label Jul 20, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #571 (9b7f0c5) into master (f873d77) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #571      +/-   ##
==========================================
- Coverage   82.47%   82.47%   -0.01%     
==========================================
  Files         167      167              
  Lines       46144    46198      +54     
==========================================
+ Hits        38059    38100      +41     
- Misses       8085     8098      +13     
Impacted Files Coverage Δ
arrow/src/array/array_list.rs 95.51% <100.00%> (+0.62%) ⬆️
arrow/src/array/equal/utils.rs 75.00% <100.00%> (ø)
arrow/src/compute/kernels/sort.rs 94.15% <100.00%> (ø)
arrow/src/array/array_struct.rs 89.11% <0.00%> (ø)
arrow-flight/src/arrow.flight.protocol.rs 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f873d77...9b7f0c5. Read the comment docs.

Copy link
Member

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @waynexia , looks great 👍

@@ -2347,7 +2347,7 @@ mod tests {
nulls_first: true,
}),
Some(3),
vec![None, None, Some(vec![Some(2)])],
vec![None, None, Some(vec![Some(1)])],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@alamb
Copy link
Contributor

alamb commented Jul 20, 2021

FYI @nevi-me and @bjchambers

@alamb alamb merged commit bd9f67a into apache:master Jul 20, 2021
@alamb
Copy link
Contributor

alamb commented Jul 20, 2021

Thanks again @waynexia

alamb pushed a commit that referenced this pull request Jul 20, 2021
* pin this bug down & fix

Signed-off-by: Ruihang Xia <[email protected]>

* tidy

Signed-off-by: Ruihang Xia <[email protected]>

* add some test cases

Signed-off-by: Ruihang Xia <[email protected]>
alamb added a commit that referenced this pull request Jul 21, 2021
* pin this bug down & fix

Signed-off-by: Ruihang Xia <[email protected]>

* tidy

Signed-off-by: Ruihang Xia <[email protected]>

* add some test cases

Signed-off-by: Ruihang Xia <[email protected]>

Co-authored-by: Ruihang Xia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ListArray equality check may return wrong result
4 participants