npawar commented on a change in pull request #5326:
URL: https://github.com/apache/incubator-pinot/pull/5326#discussion_r419724574



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/data/function/DateTimeFunctions.java
##########
@@ -27,6 +32,8 @@
  */
 public class DateTimeFunctions {
 
+  private Map<String, DateTimeFormatter> _dateTimeFormatterMap = new 
HashMap<>();

Review comment:
       While this is working great for dateTimeFormat functions,  I wonder if 
we need to instead have multiple constructors (or init methods), and create 
DateTimeFunction object for every function we encounter in the schema.
   For example, say we would want to add `round(millis, 15:MINUTES)`. The logic 
for this is (millis / (15*60*1000)) * (15*60*1000). We don't want to interpret 
"15:MINUTES" on every row. Instead, we can create a DateTimeFunction that saves 
"15:MINUTES" as a member variable, and then invoke `round()` on this object.
   We can have init methods like
   initBucket(int bucket), initRounding(int rounding), initSDF(String pattern), 
etc.




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to