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

Fixed panic in division using nulls. #438

Merged
merged 1 commit into from
Sep 22, 2021
Merged

Fixed panic in division using nulls. #438

merged 1 commit into from
Sep 22, 2021

Conversation

jorgecarleitao
Copy link
Owner

Close #437

There may be other more efficient implementations, but for now having the bug fixed is more important :)

@ritchie46
Copy link
Collaborator

Maybe we could keep the old implementation as fast path in case both arrays don't have null values.

@codecov
Copy link

codecov bot commented Sep 22, 2021

Codecov Report

Merging #438 (fe81b96) into main (cdbc958) will decrease coverage by 0.02%.
The diff coverage is 22.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #438      +/-   ##
==========================================
- Coverage   80.80%   80.77%   -0.03%     
==========================================
  Files         372      372              
  Lines       22629    22637       +8     
==========================================
  Hits        18286    18286              
- Misses       4343     4351       +8     
Impacted Files Coverage Δ
src/compute/arithmetics/basic/div.rs 81.66% <22.22%> (-10.65%) ⬇️
src/types/mod.rs 28.31% <0.00%> (-0.89%) ⬇️

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 cdbc958...fe81b96. Read the comment docs.

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Sep 22, 2021
@jorgecarleitao jorgecarleitao merged commit 93c56d7 into main Sep 22, 2021
@jorgecarleitao jorgecarleitao deleted the fix_div branch September 22, 2021 17:33
@Dandandan
Copy link
Collaborator

Maybe we could keep the old implementation as fast path in case both arrays don't have null values.

Great idea. Only needs no nulls on the right side I think (as the panic is for division by zero).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integer divsion might panic if null values in array: unchecked division
3 participants