Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Added support for compute to BinaryArray #346

Merged
merged 2 commits into from
Sep 3, 2021
Merged

Added support for compute to BinaryArray #346

merged 2 commits into from
Sep 3, 2021

Conversation

zhyass
Copy link
Contributor

@zhyass zhyass commented Aug 26, 2021 •
edited by jorgecarleitao
Loading

This PR improves the support of compute to BinaryArray. Main additions:

  • comparison
  • sort
  • contains
  • like
  • cast
  • substring
  • min_max

Closes #345

@zhyass zhyass marked this pull request as draft August 26, 2021 15:06
@codecov
Copy link

codecov bot commented Aug 26, 2021 •
edited
Loading

Codecov Report

Merging #346 (a7bc6a3) into main (cef0dce) will decrease coverage by 0.01%.
The diff coverage is 79.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #346      +/-   ##
==========================================
- Coverage   81.05%   81.03%   -0.02%     
==========================================
  Files         326      329       +3     
  Lines       21103    21734     +631     
==========================================
+ Hits        17105    17613     +508     
- Misses       3998     4121     +123
Impacted Files Coverage Δ
src/compute/comparison/utf8.rs 67.41% <ø> (ø)
src/compute/cast/binary_to.rs 75.86% <50.00%> (-24.14%) ⬇️
src/array/binary/mutable.rs 64.86% <54.71%> (-2.23%) ⬇️
src/array/binary/from.rs 75.00% <63.63%> (-25.00%) ⬇️
src/compute/comparison/binary.rs 67.41% <67.41%> (ø)
src/compute/contains.rs 50.73% <71.73%> (+10.73%) ⬆️
src/compute/sort/binary.rs 75.00% <75.00%> (ø)
src/compute/like.rs 60.39% <75.64%> (+60.39%) ⬆️
src/compute/aggregate/min_max.rs 79.13% <76.31%> (-0.45%) ⬇️
src/array/ord.rs 66.33% <83.33%> (+1.07%) ⬆️
... and 27 more

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 cef0dce...a7bc6a3. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Looks great, thanks!

I think that this shows well that we cpuld benefit from some common code for binary and utf8.

Regardless, could you add some tests in tests/?

@zhyass
Copy link
Contributor Author

zhyass commented Aug 26, 2021

Looks great, thanks!

I think that this shows well that we cpuld benefit from some common code for binary and utf8.

Regardless, could you add some tests in tests/?

Sure, I will add test cases.

@jorgecarleitao jorgecarleitao added the enhancement An improvement to an existing feature label Aug 26, 2021
@jorgecarleitao
Copy link
Owner

IMO this is ready to review (and merge ^_^)

@zhyass zhyass force-pushed the compute_binary branch 2 times, most recently from 27c47d7 to df546be Compare August 31, 2021 10:56
@zhyass
Copy link
Contributor Author

zhyass commented Aug 31, 2021

IMO this is ready to review (and merge ^_^)

Work in progress, I will complete soon.

@zhyass zhyass force-pushed the compute_binary branch 4 times, most recently from 2e3de5a to 23682f0 Compare September 2, 2021 08:40
@zhyass zhyass marked this pull request as ready for review September 2, 2021 08:41
@zhyass
Copy link
Contributor Author

zhyass commented Sep 2, 2021

IMO this is ready to review (and merge ^_^)

Ready for review now.

@zhyass
Add support for binary compute (jorgecarleitao#345)
b4befc8 
[summary]
1. comparison
2. sort
3. contains
4. like
5. cast
6. substring
7. min_max
8. cargo fmt
@zhyass zhyass force-pushed the compute_binary branch 2 times, most recently from 815e10b to 75e9156 Compare September 2, 2021 12:32
@zhyass zhyass mentioned this pull request Sep 2, 2021
Merged
4 tasks
jorgecarleitao
jorgecarleitao approved these changes Sep 2, 2021
View reviewed changes
Copy link
Owner

@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.

Amazing work, @zhyass! Went through this carefully and have only very tiny improvements, looks pretty solid, including tests.

src/array/binary/mutable.rs Outdated Show resolved Hide resolved
src/array/binary/mutable.rs Outdated Show resolved Hide resolved
src/array/binary/mutable.rs Outdated Show resolved Hide resolved
src/array/binary/mutable.rs Outdated Show resolved Hide resolved
src/array/binary/mutable.rs Outdated Show resolved Hide resolved
src/compute/contains.rs Outdated Show resolved Hide resolved
@jorgecarleitao jorgecarleitao added feature A new feature and removed enhancement An improvement to an existing feature labels Sep 3, 2021
@jorgecarleitao jorgecarleitao changed the title Add support for binary compute Added support for compute to BinaryArray Sep 3, 2021
@jorgecarleitao jorgecarleitao merged commit d6879ba into jorgecarleitao:main Sep 3, 2021
@zhyass zhyass deleted the compute_binary branch September 3, 2021 09:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jorgecarleitao jorgecarleitao jorgecarleitao approved these changes

Assignees
No one assigned
Labels
feature A new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for binary compute
2 participants
@zhyass @jorgecarleitao