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


##########
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:
   it would be an overengineering IMO --> since we dont have UDAF. 
   
   if any changes we do here creating the operator wrapped in class with 
annotations we might not have all the primitives ready for UDAF in the future. 
thus i haven't make the changes; 
   
   but regardless we can do that in 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: 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