walterddr commented on code in PR #9602:
URL: https://github.com/apache/pinot/pull/9602#discussion_r997127499
##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -140,8 +133,30 @@ public static boolean containsFunction(String
functionName) {
*/
@Nullable
public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
- Map<Integer, FunctionInfo> functionInfoMap =
FUNCTION_INFO_MAP.get(canonicalize(functionName));
- return functionInfoMap != null ? functionInfoMap.get(numParameters) : null;
+ Collection<Map.Entry<String, Function>> allFunctions =
FUNCTION_MAP.range(canonicalize(functionName), true);
+ if (allFunctions.isEmpty()) {
+ return null;
+ }
+
+ List<Function> matchingFunctions = allFunctions
+ .stream()
+ .map(Map.Entry::getValue)
+ .filter(fun -> fun.getParameters().size() == numParameters)
+ .collect(Collectors.toList());
Review Comment:
yeah technically it should be fine as you say - but it is not once per
query, it could be `O(#_of_column * #_of_function_usage)`
I think your goal is to remove the parameter-number-based mapper altogether.
can we
- keep the static final object so the lookup is fast.
- remove all access so that no one can directly modify this function map?
once we upgrade all code lookup to be type-based we can remove them all
together.
--
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]
