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

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

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

ritchie46
Copy link
Collaborator

@ritchie46 ritchie46 commented Sep 3, 2023

Reverts #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.

…y instead of the hash->hash map (#1555)"

This reverts commit cf9ec83.
@codecov
Copy link

codecov bot commented Sep 3, 2023

Codecov Report

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

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

Additional details and impacted files
@@            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 Coverage Δ
src/array/dictionary/mod.rs 95.32% <ø> (ø)
src/array/mod.rs 79.72% <ø> (ø)
src/array/dictionary/mutable.rs 79.13% <98.11%> (+8.64%) ⬆️
src/compute/cast/primitive_to.rs 93.04% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sundy-li sundy-li merged commit b2017d7 into main Sep 3, 2023
48 of 49 checks passed
@sundy-li sundy-li deleted the revert-1555-feature/dict-hash branch September 3, 2023 12:30
@aldanor
Copy link
Contributor

aldanor commented Sep 3, 2023

@sundy-li There is an open 'fix the unsafe part only and keep the rest' PR... #1561

aldanor added a commit to aldanor/arrow2 that referenced this pull request Sep 3, 2023
…the array instead of the hash->hash map" (jorgecarleitao#1559)"

This reverts commit b2017d7.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants