mkzung commented on issue #37476: URL: https://github.com/apache/arrow/issues/37476#issuecomment-4947058017
I had a go at this and landed somewhere different from what I wrote above. The AdaptiveUIntBuilder route does not work. It makes `MakeDictionaryBuilder` return a different builder class, and `util/converter.h`, `json/from_string.cc` and the R binding all cast the result to `DictionaryBuilder<T>`. `TestDictionaryUnifier.ChunkedArrayNestedDict`, which builds a `dictionary(uint32(), utf8())`, dies on a `checked_pointer_cast` DCHECK as soon as you try it. What works is keeping one builder class and preserving the requested signedness where the index type is reported. Indices are non-negative and the signed and unsigned types of a given width have the same layout, so it is value-preserving and free, and the width stays adaptive. Nothing outside `builder_dict.h` and `builder.cc` has to change, so R is fixed rather than broken. One consequence: `dictionary(uint64(), ...)` then really does produce uint64 indices, and `arrow_to_pandas.cc` refuses to convert those, so `to_pandas()` starts raising where it used to work by accident on int32 indices. uint8, uint16 and uint32 are unaffected, and take, filter, cast, IPC, Parquet, the compute kernels and Acero all keep working. PR shortly. -- 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]
