Jackie-Jiang commented on code in PR #13573:
URL: https://github.com/apache/pinot/pull/13573#discussion_r1674520077
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -95,108 +153,152 @@ public static void init() {
}
/**
- * Registers a method with the name of the method.
+ * Registers a {@link PinotScalarFunction} under the given canonical name.
*/
- public static void registerFunction(Method method, boolean
nullableParameters, boolean isPlaceholder,
- boolean isVarArg) {
- registerFunction(method.getName(), method, nullableParameters,
isPlaceholder, isVarArg);
+ private static void register(String canonicalName, PinotScalarFunction
function,
+ Map<String, PinotScalarFunction> functionMap) {
+ Preconditions.checkState(functionMap.put(canonicalName, function) == null,
"Function: %s is already registered",
+ canonicalName);
}
/**
- * Registers a method with the given function name.
+ * Registers a {@link FunctionInfo} under the given canonical name.
*/
- public static void registerFunction(String functionName, Method method,
boolean nullableParameters,
- boolean isPlaceholder, boolean isVarArg) {
- if (!isPlaceholder) {
- registerFunctionInfoMap(functionName, method, nullableParameters,
isVarArg);
- }
- registerCalciteNamedFunctionMap(functionName, method, nullableParameters,
isVarArg);
+ private static void register(String canonicalName, FunctionInfo
functionInfo, int numArguments,
+ Map<String, Map<Integer, FunctionInfo>> functionInfoMap) {
+ Preconditions.checkState(
+ functionInfoMap.computeIfAbsent(canonicalName, k -> new
HashMap<>()).put(numArguments, functionInfo) == null,
+ "Function: %s with %s arguments is already registered", canonicalName,
+ numArguments == VAR_ARG_KEY ? "variable" : numArguments);
}
- private static void registerFunctionInfoMap(String functionName, Method
method, boolean nullableParameters,
- boolean isVarArg) {
- FunctionInfo functionInfo = new FunctionInfo(method,
method.getDeclaringClass(), nullableParameters);
- String canonicalName = canonicalize(functionName);
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
- if (isVarArg) {
- FunctionInfo existFunctionInfo = functionInfoMap.put(VAR_ARG_KEY,
functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with variable number of parameters is already
registered", functionName);
- } else {
- FunctionInfo existFunctionInfo =
functionInfoMap.put(method.getParameterCount(), functionInfo);
- Preconditions.checkState(existFunctionInfo == null ||
existFunctionInfo.getMethod() == functionInfo.getMethod(),
- "Function: %s with %s parameters is already registered",
functionName, method.getParameterCount());
- }
- }
-
- private static void registerCalciteNamedFunctionMap(String functionName,
Method method, boolean nullableParameters,
- boolean isVarArg) {
- if (method.getAnnotation(Deprecated.class) == null) {
- FUNCTION_MAP.put(functionName, ScalarFunctionImpl.create(method));
- }
- }
-
- public static Map<String, List<Function>> getRegisteredCalciteFunctionMap() {
- return FUNCTION_MAP.map();
+ /**
+ * Returns {@code true} if the given canonical name is registered, {@code
false} otherwise.
+ */
+ public static boolean contains(String canonicalName) {
+ return FUNCTION_MAP.containsKey(canonicalName);
}
- public static Set<String> getRegisteredCalciteFunctionNames() {
- return FUNCTION_MAP.map().keySet();
+ @Deprecated
+ public static boolean containsFunction(String name) {
+ return contains(canonicalize(name));
}
/**
- * Returns {@code true} if the given function name is registered, {@code
false} otherwise.
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and argument types, or {@code null} if
+ * there is no matching method. This method should be called after the
FunctionRegistry is initialized and all methods
+ * are already registered.
*/
- public static boolean containsFunction(String functionName) {
- return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+ @Nullable
+ public static FunctionInfo lookupFunctionInfo(String canonicalName,
ColumnDataType[] argumentTypes) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(argumentTypes) : null;
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, or {@code null}
+ * Returns the {@link FunctionInfo} associated with the given canonical name
and number of arguments, or {@code null}
* if there is no matching method. This method should be called after the
FunctionRegistry is initialized and all
* methods are already registered.
+ * TODO: Move all usages to {@link #lookupFunctionInfo(String,
ColumnDataType[])}.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- if (functionInfoMap != null) {
- FunctionInfo functionInfo = functionInfoMap.get(numParameters);
- if (functionInfo != null) {
- return functionInfo;
- }
- return functionInfoMap.get(VAR_ARG_KEY);
- }
- return null;
+ public static FunctionInfo lookupFunctionInfo(String canonicalName, int
numArguments) {
+ PinotScalarFunction function = FUNCTION_MAP.get(canonicalName);
+ return function != null ? function.getFunctionInfo(numArguments) : null;
}
- private static String canonicalize(String functionName) {
- return StringUtils.remove(functionName, '_').toLowerCase();
+ @Deprecated
+ @Nullable
+ public static FunctionInfo getFunctionInfo(String name, int numArguments) {
+ return lookupFunctionInfo(canonicalize(name), numArguments);
Review Comment:
We have canonical name in multiple places, e.g. aggregate function,
transform function. I think we can revisit this in a separate PR
--
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]
