Improved framework for writing parametric functions #3926
Conversation
SqlScalarFunctionBuilder can be used to define scalar function with multiple specialization methods. Specialization method will be selected based on concrete java type parameters, return type and optionally predicate. It is also possible to define extra parameters that will be passed to specialization method.
|
Just general comment. A bit more verbose commit messages, explaining what was changed by the commit and why, would be very appreciated. It is great help for us (external devs) when we read the codebase. |
|
Addressed your comments, added some details to the commit message, and also added a section to the Developer Guide. Let me know how that looks! |
cberner
force-pushed
the
parametric
branch
3 times, most recently
from
f8f334d
to
0de023d
Compare
|
Updated to remove |
|
Updated with comments |
| * ``@TypeParameter``: | ||
|
|
||
| The ``@TypeParameter`` annotation is used to declare a type parameter which can | ||
| be used in the argument types ``@SqlType`` annotaion, or return type of the function. |
|
Updated to add support for implementations which are instance methods |
| return methods.build(); | ||
| } | ||
|
|
||
| private static Implementation parseImplementation(String name, Method method, Constructor<?>[] constructors) |
There was a problem hiding this comment.
Do you think this function is too long? I considered breaking it up, but because of all the different lists that have to be built it would be difficult to do without having the helper methods use out variables
There was a problem hiding this comment.
You may consider extracting private, builder-like class ImplementationParser. And then store intermediate state as fields.
Then parseImplementation(...) would look like:
private static Implementation parseImplementation(String name, Method method, Constructor<?>[] constructors) {
return new ImplementationParser(name, method, constructors).parse();
}And ImplementatinoParser.parse() would call private mathods of ImplementationParser internally.
I do not find this very elegant - but may be still more readable than functional approach with all the lists passed around from method to method.
|
Refactored to introduce an |
|
Looks good to me. |
|
@martint are we good to merge this? |
SqlScalarFunctionBuilder can be used to define scalar function with multiple specialization methods. Specialization method will be selected based on concrete java type parameters, return type and optionally predicate. It is also possible to define extra parameters that will be passed to specialization method.
|
Go for it, but check the travis failures first. |
For datatypes with multiple binary representations we need to define same function multiple times depending on actal types of parameters. E.g. add (Slice a, Slice b) and add (long a, long b) represent + operator for DECIMAL type. First version for longer DECIMAL values (represented internally as Slice) and second, faster version for shorter decimals (represented internally as long). This patch allows multiple versions of same scalar udf/operator to be defined using annotation syntax. Functions with same name are grouped together and exposed as single ParametricFunction which internally does dispatching to proper MethodHandle based on actual parameter types at runtime.
cberner
force-pushed
the
parametric
branch
2 times, most recently
from
0558fa0
to
2966972
Compare
This allows parametric scalar functions to be written in an annotation based way, similar to other scalar functions.
This makes array_distinct(array<bigint>) twice as efficient
Optimize greater_than operator for arrays with elements that use long as their native container type. This improves efficency by 2-3x
No description provided.