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

Fixed error in reading negative decimals from parquet #679

Merged
merged 1 commit into from
Dec 13, 2021
Merged

Fixed error in reading negative decimals from parquet #679

merged 1 commit into from
Dec 13, 2021

Conversation

mdrach
Copy link
Contributor

@mdrach mdrach commented Dec 13, 2021

Fixes #676 . On conversion from a FixedLenByteArray to i128, any extension bytes must be filled with the value of the MSB to correctly handle negative values.

@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #679 (2fd7a87) into main (6ec9cf5) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 2fd7a87 differs from pull request most recent head 8c5e887. Consider uploading reports for the commit 8c5e887 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   69.60%   69.61%   +0.01%     
==========================================
  Files         301      301              
  Lines       16762    16768       +6     
==========================================
+ Hits        11667    11673       +6     
  Misses       5095     5095              
Impacted Files Coverage Δ
src/io/parquet/read/mod.rs 44.23% <100.00%> (+0.81%) ⬆️
src/io/parquet/read/utils.rs 78.94% <0.00%> (ø)
src/compute/arithmetics/mod.rs 69.04% <0.00%> (ø)
src/compute/arithmetics/basic/div.rs 77.96% <0.00%> (ø)
src/compute/arithmetics/basic/mod.rs 0.00% <0.00%> (ø)
src/compute/arithmetics/basic/pow.rs 100.00% <0.00%> (ø)
src/compute/arithmetics/basic/rem.rs 84.90% <0.00%> (ø)
src/io/parquet/read/binary/basic.rs 68.32% <0.00%> (+0.60%) ⬆️

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 6ec9cf5...8c5e887. Read the comment docs.

@jorgecarleitao
Copy link
Owner

Thanks a lot, @mdrach , awesome fix. Are you working on top of crates.io? I can do a small release with this fix to 0.8.

@jorgecarleitao jorgecarleitao changed the title Correctly handle negative decimals Fixed error in reading negative decimals from parquet Dec 13, 2021
@jorgecarleitao jorgecarleitao merged commit 6e35903 into jorgecarleitao:main Dec 13, 2021
@jorgecarleitao jorgecarleitao added the bug Something isn't working label Dec 13, 2021
@mdrach
Copy link
Contributor Author

mdrach commented Dec 13, 2021

Thanks a lot, @mdrach , awesome fix. Are you working on top of crates.io? I can do a small release with this fix to 0.8.

We're using 0.7 right now. Upgrade is blocked on resolving this issue so no rush here.

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.

Parquet reading cannot handle negative values of type Decimal
2 participants