[GitHub] [arrow] hermanschaaf commented on pull request #34806: GH-34790: [Go]: Add array.Diff()

["hermanschaaf (via GitHub)" <gi...@apache.org>]

hermanschaaf commented on PR #34806:
URL: https://github.com/apache/arrow/pull/34806#issuecomment-1496264783

   @zeroshade Thank you for your review earlier, this is now ready for another look. I addressed most of your comments in https://github.com/apache/arrow/pull/34806/commits/3f45bac67f9b3e10374ba048aa2d3a1cfd6c08c6 (with a further fix in https://github.com/apache/arrow/pull/34806/commits/ebf70fd8844dc4e0d0c3be6ad3eceaa605833f1c), and switched to native Go return types in https://github.com/apache/arrow/pull/34806/commits/1f15a10f97ac24c07859a603d4dc7f4e19305e07. This way we can more easily compare the `*Struct` / Native Go implementations.
   
   Personally I prefer the native Go return types. It means we don't need to pass a memory allocator to the `Diff` function, and it simplifies the code here a bit. I toyed with introducing an `EditScript` struct and returning that, but that would have the downside of introducing a new public `array.EditScript` type, which I feel is unnecessary in this instance. It seems better to me to return `inserts []bool, runLengths []int64` and document what they contain. 
   
   Let me know your thoughts!


-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org