scovich opened a new pull request, #7888: URL: https://github.com/apache/arrow-rs/pull/7888
# Which issue does this PR close? - Closes https://github.com/apache/arrow-rs/issues/7831 # Rationale for this change Variants naturally work with `u32` offsets, field ids, etc. Widening them artificially to `usize` on 64-bit architectures causes several problems: 1. A majority of developers will be using 64-bit architectures, and are unlikely to think about integer overflow issues when working with `usize`. But it's actually quite easy for malicious data or buggy code to overflow the `u32` values that variant actually relies on. Worse, it becomes difficult, if not impossible, to validate the code's resistance to 32-bit integer overflow, when manipulating `usize` values on 64-bit hardware. 2. Related to 1/, casting from `usize` to `u32` can clip the value on 64-bit hardware, which makes it harder to reason about the code's correctness (always wondering whether the value _might_ be larger than 32-bits can hold). In contrast, casting from `u32` to `usize` is safe in spite of being fallible in rust (assumes we do _not_ need to support 16-bit architectures). 3. The variant-related data structures occupy significantly more space than they need to, when storing (64-bit) `usize` offsets instead of `u32`. # What changes are included in this PR? Store all variant-related offsets as `u32` instead of `usize`. The `VariantMetadata`, `VariantObject` and `VariantList` structs shrink to 32/64/64 bytes (previously 40/88/80 bytes). # Are these changes tested? Existing unit tests cover the use of these values; new static assertions will catch any future size changes. # Are there any user-facing changes? No. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
