jorgecarleitao / arrow2

Transmute-free Rust library to work with the Arrow format
Apache License 2.0
1.07k stars 223 forks source link

Fixed timestamp to datetime conversion for values with fractional seconds before the epoch #1467

Closed alexander-beedie closed 1 year ago

alexander-beedie commented 1 year ago

The timestamp_XX_to_datetime functions all panic with "invalid or out-of-range datetime" on conversions of values with fractional seconds before the epoch (where the timestamp becomes negative).

For these timestamps (pre-epoch, fractional secs) the (v % <scale_factor>) as u32 formulation is not valid, as

This PR allows pre-epoch conversions with fractional seconds to be handled correctly, and adds some new tests to provide the missing coverage.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 :tada:

Comparison is base (e14c238) 83.92% compared to head (888f780) 83.94%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1467 +/- ## ========================================== + Coverage 83.92% 83.94% +0.01% ========================================== Files 387 387 Lines 41605 41620 +15 ========================================== + Hits 34917 34936 +19 + Misses 6688 6684 -4 ``` | [Impacted Files](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1467?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | | |---|---|---| | [src/temporal\_conversions.rs](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1467?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL3RlbXBvcmFsX2NvbnZlcnNpb25zLnJz) | `91.39% <100.00%> (+0.44%)` | :arrow_up: | ... and [4 files with indirect coverage changes](https://codecov.io/gh/jorgecarleitao/arrow2/pull/1467/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

ritchie46 commented 1 year ago

Thanks @alexander-beedie. Great fix. :+1: