geoffreyclaude commented on PR #23013:
URL: https://github.com/apache/datafusion/pull/23013#issuecomment-4876283145

   > Thank you @geoffreyclaude and @kosiew
   > 
   > I played around with this code and I have an alternate suggestion that I 
think is simpler and provides the same functionality
   > 
   > * #23299
   > 
   > Let me know what you think
   
   @alamb I agree there's room for improvement here, the solution I came up 
with is probably too complex. Your #23299 is simple and works well for Int8 and 
Int16, but I'm worried it won't scale well to all the other types we'll need to 
similarly reinterpret for the future optims: 1 byte types to Uint8, 2 byte 
types to Uint16, 4 byte types to Uint32, 8 byte types to UInt64, and 16 byte 
types to UInt128. Hardcoding all the mappings doesn't seem right.
   
   Since it's usually a bad idea to generalize before the need is clear, I 
suggest we go with your approach, then use the next  "branchless" optim PR 
#23014 to discuss the "ideal" generalization?


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to