New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Support function registry with arg types #9397
base: master
Are you sure you want to change the base?
Conversation
61yao
commented
Sep 14, 2022
- Functions are registered using FunctionName and DataType (converted from Java type)
- To make this fully work, we have to support all literal passing from SqlNode to Thrift so we can support all LiteralType
* <p>TODO: Merge FunctionRegistry and FunctionDefinitionRegistry to provide one single registry for all functions. | ||
*/ | ||
public class FunctionRegistry { | ||
private FunctionRegistry() { | ||
} | ||
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(FunctionRegistry.class); | ||
|
||
// Map from function name to parameter types to function info. | ||
private static final Map<String, Map<List<FieldSpec.DataType>, FunctionInfo>> FUNC_PARAM_INFO_MAP = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest making the value a separate class, which can be used to look up the FunctionInfo
with a list of argument types. That way we don't need to maintain 2 maps, and can support type matching (match int argument to long parameter function) in the future.
* <p>TODO: Merge FunctionRegistry and FunctionDefinitionRegistry to provide one single registry for all functions. | ||
*/ | ||
public class FunctionRegistry { | ||
private FunctionRegistry() { | ||
} | ||
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(FunctionRegistry.class); | ||
|
||
// Map from function name to parameter types to function info. | ||
private static final Map<String, Map<List<FieldSpec.DataType>, FunctionInfo>> FUNC_PARAM_INFO_MAP = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to use ColumnDataType
here which contains the SV/MV info
@@ -92,11 +138,17 @@ public static void registerFunction(Method method, boolean nullableParameters) { | |||
* Registers a method with the given function name. | |||
*/ | |||
public static void registerFunction(String functionName, Method method, boolean nullableParameters) { | |||
FunctionInfo functionInfo = new FunctionInfo(method, method.getDeclaringClass(), nullableParameters); | |||
String canonicalName = canonicalize(functionName); | |||
final FunctionInfo functionInfo = new FunctionInfo(method, method.getDeclaringClass(), nullableParameters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(convention) We don't usually put final
for local variables
This one relies on #9389. We can first finish that and get back to this |
SG |