Jackie-Jiang commented on a change in pull request #8248:
URL: https://github.com/apache/pinot/pull/8248#discussion_r815034844



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -651,7 +651,8 @@ public static long dateTrunc(String unit, long timeValue, 
String inputTimeUnitSt
    * @return truncated timeValue
    *
    */
-  @ScalarFunction
+  // This function name is registered by DateTruncTransformationFunction, so 
rename to dateTruncScalar.

Review comment:
       We intentionally keep both methods the same name. Scalar function can be 
used for ingestion transform, constant calculation and post aggregation. For 
regular transform, we will pick `TransformFunction` over scalar function if 
both versions exist.

##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -651,7 +651,8 @@ public static long dateTrunc(String unit, long timeValue, 
String inputTimeUnitSt
    * @return truncated timeValue
    *
    */
-  @ScalarFunction
+  // This function name is registered by DateTruncTransformationFunction, so 
rename to dateTruncScalar.
+  @ScalarFunction(names = "dateTruncScalar", returnType = "TIMESTAMP")

Review comment:
       Not sure if we need `returnType` here. Changing the return value for 
this method to `Timestamp` can serve the purpose. See `toTimestamp()` as an 
example. Note that this is backward-incompatible change

##########
File path: 
pinot-spi/src/main/java/org/apache/pinot/spi/annotations/ScalarFunction.java
##########
@@ -46,5 +46,10 @@
 
   boolean enabled() default true;
 
-  String name() default "";
+  // A comma-separated function names to register in FunctionRegistry
+  String names() default "";
+
+  String alias() default "";

Review comment:
       +1 on Richard's suggestion. Also since we allow multiple names, alias 
seems redundant




-- 
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