theirix commented on PR #19874: URL: https://github.com/apache/datafusion/pull/19874#issuecomment-3796809826
> I'm having trouble understanding the rationale here; `log`, `power` and `round` at most have one decimal input, and only `round` preserves the decimal type whereas the others will return floats anyway. So all this handling for getting precision/scale of left/right inputs, adjusting the scale of the output decimal, seems unused? > For most of these functions, agree, only one argument is decimal, so we execute the decimal/non-decimal case with code for adjusting both scales untouched. Overall, I can see the following benefits: - simplifying a caller code by duplicating non-obvious logic to this helper - for decimal/non-decimal case, it performs casting of input and output types, so parameter scale is not lost when operating on a native type - for decimal outputs, it removes the burden of setting the precision and scale on output - for array cases, it is hard to write manual code of scaling to output type (not always a default `Decimal(38,10)`), so `cast_array_to` does it for the caller - providing effective scale and precision of a type to the user-provided kernel without capturing it from arguments So, symmetric functions like gcd/lcm (WIP), mod, div, etc benefit most from this PR. From the first glance, a `log` function can be greatly simplified by dropping `unscale_to_*` calls in kernels and extra casting. For `pow`, originally it was Dec x Float -> Dec, but since introducing a fallback to the float version from #19369 (still thinking when it is necessary), it is less relevant. > Also for `round` we have a PR relating to altering precision/scale: > > * [fix: increase ROUND decimal precision to prevent overflow truncation #19926](https://github.com/apache/datafusion/pull/19926) Yes, I discovered it recently. I am wondering whether it could also be simplified using this PR, since it handles different input and output precisions and scales. -- 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]
