You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Review the need of make_scalar_function for functions #14835
Is your feature request related to a problem or challenge?
The current code converts scalars to arrays and then back after the function calculation. However, this conversion is unnecessary and can be optimized, especially in cases involving scalars. The conversion process duplicates values in the array, which doesn't add any value.
This approach was likely implemented when Scalar was not yet introduced in arrow-rs, but that’s no longer the case. With Scalar available, the conversion to arrays is redundant.
For example, the gcd function does not require an array and can instead operate directly on an i64 value #14834
Describe the solution you'd like
Revisit the functions that use make_scalar_function and identify those where it is no longer necessary. If there are functions that no longer require it, remove the usage entirely to streamline the code.
Revisit the functions that use make_scalar_function and identify those where it is no longer necessary. If there are functions that no longer require it, remove the usage entirely to streamline the code.
I think trying out @findepi 's Simple Functions #12635 would be an excellent candidate for these functions. The ones that use make_scalar_function are all fairly simple and would benefit from having a special case for constant.
Ensuring #12635 can handle the basic math functions would be a good validation of the API in general
I think trying out @findepi 's Simple Functions #12635 would be an excellent candidate for these functions. The ones that use make_scalar_function are all fairly simple and would benefit from having a special case for constant.
I think trying out @findepi 's Simple Functions #12635 would be an excellent candidate for these functions. The ones that use make_scalar_function are all fairly simple and would benefit from having a special case for constant.
I agree, but it is still far from ready.
i am not questioning it (i know very well how much more I'd like to do there), but I am curious about outsider's perspective.
in your eyes, what makes you say "far from ready", @jayzhan211 ?
To replace the existing function without needing to maintain it, we must ensure that all its current functionality is fully supported. Among everything we have so far, extensibility seems to be the biggest challenge. The more extensible we make the function, the less simple it becomes. Since I haven't seen a viable solution for this in #14668, I believe it is still far from being ready.
I think trying out @findepi 's Simple Functions #12635 would be an excellent candidate for these functions. The ones that use make_scalar_function are all fairly simple and would benefit from having a special case for constant.
I agree, but it is still far from ready.
i am not questioning it (i know very well how much more I'd like to do there), but I am curious about outsider's perspective. in your eyes, what makes you say "far from ready", @jayzhan211 ?
FWIW in my opinion, getting to the level of replacing make_scalar_function (and improving the resulting implementations) seems like a good first step towards a more general solution for different types of functions
Activity
alamb commentedon Feb 23, 2025
make_scalar_functionare all fairly simple and would benefit from having a special case for constant.Ensuring #12635 can handle the basic math functions would be a good validation of the API in general
jayzhan211 commentedon Feb 24, 2025
I agree, but it is still far from ready.
jayzhan211 commentedon Feb 24, 2025
it seems this issue is accidentally added in blog sub-issue
alamb commentedon Feb 24, 2025
Fixed -- sorry about that -- I am not sure how I did that but will be more careful
findepi commentedon Feb 24, 2025
i am not questioning it (i know very well how much more I'd like to do there), but I am curious about outsider's perspective.
in your eyes, what makes you say "far from ready", @jayzhan211 ?
jayzhan211 commentedon Feb 24, 2025
To replace the existing function without needing to maintain it, we must ensure that all its current functionality is fully supported. Among everything we have so far, extensibility seems to be the biggest challenge. The more extensible we make the function, the less simple it becomes. Since I haven't seen a viable solution for this in #14668, I believe it is still far from being ready.
alamb commentedon Feb 24, 2025
FWIW in my opinion, getting to the level of replacing
make_scalar_function(and improving the resulting implementations) seems like a good first step towards a more general solution for different types of functions