taepper commented on PR #51396: URL: https://github.com/apache/arrow/pull/51396#issuecomment-6493994578
I am not an Arrow member, so feel free to disregard my point-of-view. But in my opinion, for the example to be reusable by other code it should be a direct conversion to arrow structures, instead of doing a row-wise buffering into `std::vector`, which might not be possible for the user's use-case. So even the "columnar" conversion is actually a row-wise implementation. But instead of directly writing to arrow buffers (`Builder` types), we instead buffer first into `std::vector` (and this is not a simple buffering, but already recursively walking the entire documents) and we delay the actual conversion to later. Additionally, the custom `JsonValue` type makes it easier to reuse parts of our example code. I pushed an example refactoring here: https://github.com/taepper/arrow/blob/5247184017656960f6615a4f3befca1ca0f6fe9b/cpp/examples/arrow/json_to_arrow_converter.cc#L60-L168 Note that the actual conversion code got a lot briefer. Feel free to cherry pick (parts of) the changes. Yet again, I do not want to block this PR unnecessarily, so if other reviewers feel differently, feel free to disregard -- 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]
