HuaHuaY commented on PR #51325: URL: https://github.com/apache/arrow/pull/51325#issuecomment-5717378521
> NaNs should just be compared by physical value I checked arrow-rs. Its behavior seems inconsistent with this repository; it uses `!=` to compare `NaN` values, so it does not merge any `NaN`s, even if their physical values are identical. It doesn't have a method like `AppendScalar `; based on the example in the issue, it seems to get 6 run-ends. https://github.com/apache/arrow-rs/blob/0a5979560ac67135134c3618519b9b7384edf4d2/arrow-array/src/builder/primitive_run_builder.rs#L176 If we want to allow the merging of `NaN` values, I agree with your point. I think we should compare the physical values. And `cpp/src/arrow/compute/kernels/vector_run_end_encode.cc` says > // Avoid merging floating-point values whose representations may differ. Signed > // zeros compare unequal, and NaNs remain in separate runs to preserve payloads. While checking arrow's code, I discovered another issue. When constructing dictionary arrays for Parquet in #50807, I compare the physical values of NaNs, but Arrow's dictionary builder does not do this (see `cpp/src/arrow/util/hashing.h`). Perhaps we could open a separate PR to fix that. -- 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]
