gortiz commented on code in PR #12302:
URL: https://github.com/apache/pinot/pull/12302#discussion_r1463217848


##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -94,83 +154,195 @@ private FunctionRegistry() {
   public static void init() {
   }
 
+  @VisibleForTesting
+  public static void registerFunction(Method method) {
+    registerFunction(method, Collections.singleton(method.getName()), false, 
false);
+  }
+
+  @VisibleForTesting
+  public static Set<String> getRegisteredCalciteFunctionNames() {
+    return getFunctionMap().map().keySet();
+  }
+
+  /**
+   * Returns {@code true} if the given function name is registered, {@code 
false} otherwise.
+   */
+  public static boolean containsFunction(String functionName) {
+    return getFunctionMap().containsKey(functionName, CASE_SENSITIVITY);
+  }
+
   /**
-   * Registers a method with the name of the method.
+   * 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.
    */
-  public static void registerFunction(Method method, boolean 
nullableParameters, boolean isPlaceholder,
-      boolean isVarArg) {
-    registerFunction(method.getName(), method, nullableParameters, 
isPlaceholder, isVarArg);
+  @Nullable
+  public static FunctionInfo getFunctionInfo(String functionName, int 
numParams) {
+    return getFunctionInfoFromCalciteNamedMap(functionName, numParams);
   }
 
   /**
-   * Registers a method with the given function name.
+   * 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.
    */
-  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 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);
+  @Nullable
+  public static FunctionInfo getFunctionInfo(SqlOperatorTable operatorTable, 
RelDataTypeFactory typeFactory,
+      String functionName, List<DataSchema.ColumnDataType> argTypes) {
+    PinotScalarFunction scalarFunction = getScalarFunction(operatorTable, 
typeFactory, functionName, argTypes);
+    if (scalarFunction != null) {
+      return scalarFunction.getFunctionInfo();
     } 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());
+      // TODO: convert this to throw IAE when all operator has scalar 
equivalent.
+      return null;
     }
   }
 
-  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));
+  @Nullable
+  private static FunctionInfo getFunctionInfoFromCalciteNamedMap(String 
functionName, int numParams) {
+    List<PinotScalarFunction> candidates = 
getFunctionMap().range(functionName, CASE_SENSITIVITY).stream().filter(
+            e -> e.getValue() instanceof PinotScalarFunction && 
(e.getValue().getParameters().size() == numParams
+                || ((PinotScalarFunction) e.getValue()).isVarArgs())).map(e -> 
(PinotScalarFunction) e.getValue())
+        .collect(Collectors.toList());

Review Comment:
   This would be easier to read with a fluent notation like:
   ```java
       List<PinotScalarFunction> candidates = getFunctionMap()
          .range(functionName, CASE_SENSITIVITY)
          .stream()
          .filter(e -> e.getValue() instanceof PinotScalarFunction)
          .map(e -> (PinotScalarFunction) e.getValue())
          .filter(fun.getParameters().size() == numParams || fun.isVarArgs())
          .collect(Collectors.toList());
   ```



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to