kishansairam9 commented on a change in pull request #7877:
URL: https://github.com/apache/pinot/pull/7877#discussion_r765368229
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
##########
@@ -106,14 +107,18 @@ public static boolean containsFunction(String
functionName) {
}
/**
- * Returns the {@link FunctionInfo} associated with the given function name
and number of parameters, 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.
+ * Returns a pair <{@code true}, {@link FunctionInfo}> associated with the
given function name and number of
+ * parameters if it exists, <{@code true}, {@code null}> if method exists
but number of parameters do not match,
+ * else if there is no matching method <{@code false}, {@code null}>. This
method should be called after the
+ * FunctionRegistry is initialized and all methods are already registered.
*/
@Nullable
- public static FunctionInfo getFunctionInfo(String functionName, int
numParameters) {
+ public static ImmutablePair<Boolean, FunctionInfo> getFunctionInfo(String
functionName, int numParameters) {
Review comment:
I felt that using `containsFunction` might improve readability and
changes are as in commit
https://github.com/apache/pinot/commit/8865d2e14d8dc6de4e84ddcfca7aae9e29dc5364
.
But when it comes to overhead, it might be more than only when it returns
`null` due to additional pre-checks in certain cases. Specifically in
`InbuiltFunctionEvaluator.planExecution` and constructor of
`PostAggregationFunction`.
Could you comment on performance implications of additional map lookup in
these places? If it can be significant, I will resort to creating a new
exception.
--
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]
