Re: [PR] [multistage][feature] FunctionRegistry unification [pinot]

["gortiz (via GitHub)" <gi...@apache.org>]

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


##########
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java:
##########
@@ -68,22 +84,67 @@ private FunctionRegistry() {
       }
       ScalarFunction scalarFunction = method.getAnnotation(ScalarFunction.class);
       if (scalarFunction.enabled()) {
-        // Annotated function names
-        String[] scalarFunctionNames = scalarFunction.names();
+        // Parse annotated function names and alias
+        Set<String> scalarFunctionNames = Arrays.stream(scalarFunction.names()).collect(Collectors.toSet());
+        if (scalarFunctionNames.size() == 0) {
+          scalarFunctionNames.add(method.getName());
+        }
+        boolean nullableParameters = scalarFunction.nullableParameters();
+        boolean varArg = scalarFunction.isVarArg();
+        registerFunction(method, scalarFunctionNames, nullableParameters, varArg);
+      }
+    }
+    Set<Class<?>> classes =
+        PinotReflectionUtils.getClassesThroughReflection(".*\\.function\\..*", ScalarFunction.class);
+    for (Class<?> clazz : classes) {
+      if (!Modifier.isPublic(clazz.getModifiers())) {
+        continue;
+      }
+      ScalarFunction scalarFunction = clazz.getAnnotation(ScalarFunction.class);
+      if (scalarFunction.enabled()) {
+        // Parse annotated function names and alias
+        Set<String> scalarFunctionNames = Arrays.stream(scalarFunction.names()).collect(Collectors.toSet());
+        if (scalarFunctionNames.size() == 0) {
+          scalarFunctionNames.add(clazz.getName());
+        }
         boolean nullableParameters = scalarFunction.nullableParameters();
-        boolean isPlaceholder = scalarFunction.isPlaceholder();
-        boolean isVarArg = scalarFunction.isVarArg();
-        if (scalarFunctionNames.length > 0) {
-          for (String name : scalarFunctionNames) {
-            FunctionRegistry.registerFunction(name, method, nullableParameters, isPlaceholder, isVarArg);
-          }
-        } else {
-          FunctionRegistry.registerFunction(method, nullableParameters, isPlaceholder, isVarArg);
+        boolean varArg = scalarFunction.isVarArg();
+        registerFunction(clazz, scalarFunctionNames, nullableParameters, varArg);
+      }
+    }
+    LOGGER.info("Initialized FunctionRegistry with {} functions: {} in {}ms", FUNCTION_MAP.map().size(),
+        FUNCTION_MAP.map().keySet(), System.currentTimeMillis() - startTimeMs);
+
+    // REGISTER OPERATORS
+    // Walk through all the Pinot aggregation types and
+    //   1. register those that are supported in multistage in addition to calcite standard opt table.
+    //   2. register special handling that differs from calcite standard.
+    for (AggregationFunctionType aggregationFunctionType : AggregationFunctionType.values()) {

Review Comment:
   I guess is not the target of this PR but... how difficult would it be to also have operations that are read from the classpath like we do with functions?



-- 
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-unsubscribe@pinot.apache.org

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


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