You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/06/03 14:37:59 UTC

[GitHub] [arrow] bkietz commented on pull request #7341: ARROW-9022: [C++][Compute] Make Add function safe for numeric limits

bkietz commented on pull request #7341:
URL: https://github.com/apache/arrow/pull/7341#issuecomment-638241193


   > we should use `(int8, int8) -> int16`
   
   I'm not sure that's desirable. For one thing this leads to inconsistent handling of 64 bit integer types, which are currently allowed to overflow (NB: that means this kernel includes undefined behavior for int64).
   
   There are a few other approaches we could take (ordered by personal preference):
   
   - define explicit overflow behavior for signed integer operands (for example if we declared that `add(i8(a), i8(b))` will always be equivalent to `i8(i16(a) + i16(b))` then we could instantiate only unsigned addition kernels)
   - raise an error on signed overflow
   - provide `ArithmeticOptions::overflow_behavior` and allow users to choose between these
   - require users to pass arguments which will not overflow
   
   @wesm ?
   
   This is probably nuanced enough to merit a mailing list discussion.


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

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