llama90 commented on PR #41827: URL: https://github.com/apache/arrow/pull/41827#issuecomment-2133502365
I have been examining the code with the intention of using `StringFormatter`, as it seems like a reasonable choice. However, currently, `StringFormatter` mostly handles primitive types and takes time to support complex types such as `Union`, `Struct`, and `List-Like` types. Not only did I review the code, but I also attempted to modify it. These modifications are related to the Legacy code such as `CastImpl`. For reference, please see: https://github.com/apache/arrow/blob/ff9921ffa89585be69ae85674bb365d03cb22ba4/cpp/src/arrow/scalar.cc#L1239-L1250 The reason I am addressing these issues is fundamentally to remove the legacy `CastTo`. In the short term, there may be code that requires improvement, but I would like to approach this with the following steps: 1. Implement Cast Compute Kernel. - #41668 - #41669 - #41809 - #41810 2. Remove Legacy Cast. - #39182 3. Extend `StringFormatter` to support additional types (e.g., Union, Struct, List-Like, etc.). - New Issues. 4. Enable the use of `StringFormatter` within the Cast Compute Kernel. - New Issues. Even the existing implementation of `CastTo` (to string) does not utilize `StringFormatter` for complex types. For reference, please see: https://github.com/apache/arrow/blob/ff9921ffa89585be69ae85674bb365d03cb22ba4/cpp/src/arrow/scalar.cc#L1309-L1326 What do you think of this approach? I believe this is a way to address each issue and improve the code accordingly. Mentioning everyone who has reviewed the cast code. cc @kou, @felipecrv, @ZhangHuiGui -- 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
