bit2swaz commented on PR #10798: URL: https://github.com/apache/arrow-rs/pull/10798#issuecomment-5391723672
it passes on main because `arrow-array`'s `force_validate`is empty and doesnt turn on the validation that lives in `arrow-data`: https://github.com/apache/arrow-rs/blob/d6b0bd7ecc857bce0c3b0f97ccae17d5fc3726c4/arrow-array/Cargo.toml#L72 so in the `arrow-array` test job the misaligned buffer is never rejected, with or without this change. the validation only actually runs when `arrow-data/force_validate` is on which only happens thru the `arrow` meta-crate's `force_validate` (it forwards to both crates) and that job doesnt run `arrow-array`'s unit tests. so factually speaking today no CI config runs this test with the validation enabled and being honest with you: i really couldnt find a way to write a test that exercises the fix under `arrow-data/force_validate`. the test has to hand `from_ffi` a buffer thats under aligned for its type. the only way to get one is an `ArrayData` from `build_unchecked` but under `force_validate` that validates anyway (the gate ignores `skip_validation`), so the fixture is rejected at construction, before `from_ffi` runs https://github.com/apache/arrow-rs/blob/d6b0bd7ecc857bce0c3b0f97ccae17d5fc3726c4/arrow-data/src/data.rs#L2267-L2268 you cant source it from a valid array either: to be under aligned for the import type youd need a source type with the same byte width but smaller alignment and no arrow primitive fits that (`align_of == size_of`). so the misaligned input is genuinely unconstructable in a `force_validate` build what is covered: normal builds run the realignment test. under `arrow-data/force_validate` the full `ffi` test module still passes since valid arrays go through the same align-before-validate path in `consume` so the reorder doesnt break anything. the one gap is the under aligned case under `force_validate` which is the case that cant be built based on my research so i can realistically think of two options: leave it as is with the gate and the comment explaining why, or i hand build the `FFI_ArrowArray` from a raw under aligned pointer in the test to bypass `ArrayData` entirely and get real coverage, but at the cost of more unsafe test code. which would you prefer? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
