jvanstraten commented on PR #14434: URL: https://github.com/apache/arrow/pull/14434#issuecomment-1280844214
General note that applies more broadly than just this PR: I don't think `DecodeOptionlessOverflowableArithmetic` is suitable for these functions, nor for division or for any of the floating-point versions of the functions it's already used for. The problem is that these functions all have different sets of options (integer overflow, floating point rounding, and/or domain error handling), while `DecodeOptionlessOverflowableArithmetic` only seems to handle the integer overflow option. In addition, it expects `<name>_checked` kernels to exist in addition to the normal kernels to handle the "throw an error on integer overflow" option. Also, Substrait doesn't even define functions that are generally only applied to floating point numbers (like `exp`) for integers at all. That's not necessarily a problem for consumption, though. That being said, the way options are passed in Substrait is [currently being revised](https://github.com/substrait-io/substrait/pull/342), so I don't know how much value there is in fixing this short-term/maybe it's fine for now as a placeholder. I'm not at all up-to-date on planning for Acero-Substrait. -- 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]
