search

jorgecarleitao / arrow2

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

Revert "MutableDictionaryArray rewrite: use values stored in the array instead of the hash->hash map" #1559

Closed ritchie46 closed 10 months ago

ritchie46 commented 10 months ago

Reverts jorgecarleitao/arrow2#1555

This added a lot of unsafe code that I don't think we need (and miri is not happy). I put this revert ready to get the discussion going.

codecov[bot] commented 10 months ago

Codecov Report

Patch coverage: 98.18% and project coverage change: +0.12% :tada:

Comparison is base (cf9ec83) 82.95% compared to head (2a58536) 83.08%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1559 +/- ## ========================================== + Coverage 82.95% 83.08% +0.12% ========================================== Files 391 389 -2 Lines 42809 42640 -169 ========================================== - Hits 35512 35426 -86 + Misses 7297 7214 -83 ``` | [Files Changed](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao) | Coverage Δ | | |---|---|---| | [src/array/dictionary/mod.rs](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2RpY3Rpb25hcnkvbW9kLnJz) | `95.32% <ø> (ø)` | | | [src/array/mod.rs](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L21vZC5ycw==) | `79.72% <ø> (ø)` | | | [src/array/dictionary/mutable.rs](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2FycmF5L2RpY3Rpb25hcnkvbXV0YWJsZS5ycw==) | `79.13% <98.11%> (+8.64%)` | :arrow_up: | | [src/compute/cast/primitive\_to.rs](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jorge+Leitao#diff-c3JjL2NvbXB1dGUvY2FzdC9wcmltaXRpdmVfdG8ucnM=) | `93.04% <100.00%> (ø)` | | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/jorgecarleitao/arrow2/pull/1559/indirect-changes?src=pr&el=tree-more&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: Have feedback on the report? Share it here.

aldanor commented 10 months ago

@sundy-li There is an open 'fix the unsafe part only and keep the rest' PR... https://github.com/jorgecarleitao/arrow2/pull/1561