my-vegetable-has-exploded commented on PR #5217:
URL: https://github.com/apache/arrow-rs/pull/5217#issuecomment-1870320991

   > Had a review, I like where this is headed but I don't think the null mask 
handling is quite right yet.
   > 
   > FWIW I'm not sure that separating out the null mask and values comparison 
makes sense, instead I would expect the logic to just recurse across the fields 
and union the null masks of the results (if any), with a little bit of extra 
logic to handle any null mask in the struct array proper.
   
   I wanted to do that at first. The main reason is that I found it's hard to 
reuse those codes
   
   
https://github.com/apache/arrow-rs/blob/3cd6da0a6b97056f3717d3491c84ec6d734aa262/arrow-ord/src/cmp.rs#L225-L285
   
   If there is a better way to organize those codes, I'd like to have a try! 
Thanks a lot!


-- 
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]

Reply via email to