You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2020/08/14 05:00:48 UTC

[incubator-pinot] branch master updated: Add additional datetime functionality (#5438)

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 45d5d29  Add additional datetime functionality (#5438)
45d5d29 is described below

commit 45d5d29f2ef83317e6c4713af2cf2f846e2ded04
Author: Charlie Summers <ch...@gomerits.com>
AuthorDate: Thu Aug 13 22:00:36 2020 -0700

    Add additional datetime functionality (#5438)
    
    Add more datetime functionality during both ingestion and query time. It is inspired by PrestoDB's datetime functionality, detailed here: https://prestodb.io/docs/current/functions/datetime.html
    
    Also does some refactoring of FunctionRegistry to allow for function overloading.
    
    New functions include: (all functions can take optional time zone)
    - `timezone_hour`: Returns the hour of the time zone offset (time zone is mandatory)
    - `timezone_minute`: Returns the minute of the time zone offset (time zone is mandatory)
    - `year`: Returns the year from the given epoch millis
    - `year_of_week`: Returns the year of the ISO week from the given epoch millis
    - `yow`: Alias for `year_of_week`
    - `quarter`: Returns the quarter of the year from the given epoch millis
    - `month`: Returns the month of the year from the given epoch millis
    - `week`: Returns the ISO week of the year from the given epoch millis
    - `week_of_year`: Alias for `week`
    - `day_of_year`: Returns the day of the year from the given epoch millis
    - `doy`: Alias for `day_of_year`
    - `day`: Returns the day of the month from the given epoch millis
    - `day_of_month`: Alias for `day`
    - `day_of_week`: Returns the day of the week from the given epoch millis
    - `dow`: Alias for `day_of_week`
    - `hour`: Returns the hour of the day from the given epoch millis
    - `minute`: Returns the minute of the hour from the given epoch millis
    - `second`: Returns the second of the minute from the given epoch millis
    - `millisecond`: Returns the millisecond of the second from the given epoch millis
---
 .../common/function/DateTimePatternHandler.java    |   4 +-
 .../pinot/common/function/FunctionRegistry.java    |  26 +-
 .../common/function/TransformFunctionType.java     |   2 +-
 .../common/function/scalar/DateTimeFunctions.java  | 302 ++++++++++++++++++++-
 .../apache/pinot/sql/parsers/CalciteSqlParser.java |  32 ++-
 .../data/function/InbuiltFunctionEvaluator.java    |   5 +-
 .../function/TransformFunctionFactory.java         |  12 +-
 .../postaggregation/PostAggregationFunction.java   |   7 +-
 .../function/InbuiltFunctionEvaluatorTest.java     |  91 ++-----
 .../core/data/function/InbuiltFunctionsTest.java   |  77 ++++++
 10 files changed, 459 insertions(+), 99 deletions(-)

diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/DateTimePatternHandler.java b/pinot-common/src/main/java/org/apache/pinot/common/function/DateTimePatternHandler.java
index c9c0f81..08f9bff 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/DateTimePatternHandler.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/DateTimePatternHandler.java
@@ -29,7 +29,7 @@ public class DateTimePatternHandler {
   /**
    * Converts the dateTimeString of passed pattern into a long of the millis since epoch
    */
-  public static Long parseDateTimeStringToEpochMillis(String dateTimeString, String pattern) {
+  public static long parseDateTimeStringToEpochMillis(String dateTimeString, String pattern) {
     DateTimeFormatter dateTimeFormatter = getDateTimeFormatter(pattern);
     return dateTimeFormatter.parseMillis(dateTimeString);
   }
@@ -37,7 +37,7 @@ public class DateTimePatternHandler {
   /**
    * Converts the millis representing seconds since epoch into a string of passed pattern
    */
-  public static String parseEpochMillisToDateTimeString(Long millis, String pattern) {
+  public static String parseEpochMillisToDateTimeString(long millis, String pattern) {
     DateTimeFormatter dateTimeFormatter = getDateTimeFormatter(pattern);
     return dateTimeFormatter.print(millis);
   }
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
index 58be240..e9c4b65 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.common.function;
 
+import com.google.common.base.Preconditions;
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
@@ -43,7 +44,7 @@ public class FunctionRegistry {
   }
 
   private static final Logger LOGGER = LoggerFactory.getLogger(FunctionRegistry.class);
-  private static final Map<String, FunctionInfo> FUNCTION_INFO_MAP = new HashMap<>();
+  private static final Map<String, Map<Integer, FunctionInfo>> FUNCTION_INFO_MAP = new HashMap<>();
 
   /**
    * Registers the scalar functions via reflection.
@@ -91,17 +92,28 @@ public class FunctionRegistry {
    */
   public static void registerFunction(String functionName, Method method) {
     FunctionInfo functionInfo = new FunctionInfo(method, method.getDeclaringClass());
-    FUNCTION_INFO_MAP.put(canonicalize(functionName), functionInfo);
+    String canonicalName = canonicalize(functionName);
+    Map<Integer, FunctionInfo> functionInfoMap = FUNCTION_INFO_MAP.computeIfAbsent(canonicalName, k -> new HashMap<>());
+    Preconditions.checkState(functionInfoMap.put(method.getParameterCount(), functionInfo) == null,
+        "Function: %s with %s parameters is already registered", functionName, method.getParameterCount());
   }
 
   /**
-   * Returns the {@link FunctionInfo} associated with the given function name, or {@code null} if there is no method
-   * registered under the name. This method should be called after the FunctionRegistry is initialized and all methods
-   * are already registered.
+   * Returns {@code true} if the given function name is registered, {@code false} otherwise.
+   */
+  public static boolean containsFunction(String functionName) {
+    return FUNCTION_INFO_MAP.containsKey(canonicalize(functionName));
+  }
+
+  /**
+   * 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.
    */
   @Nullable
-  public static FunctionInfo getFunctionByName(String functionName) {
-    return FUNCTION_INFO_MAP.get(canonicalize(functionName));
+  public static FunctionInfo getFunctionInfo(String functionName, int numParameters) {
+    Map<Integer, FunctionInfo> functionInfoMap = FUNCTION_INFO_MAP.get(canonicalize(functionName));
+    return functionInfoMap != null ? functionInfoMap.get(numParameters) : null;
   }
 
   private static String canonicalize(String functionName) {
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java b/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
index 65cf1df..0893916 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/TransformFunctionType.java
@@ -94,7 +94,7 @@ public enum TransformFunctionType {
     try {
       return TransformFunctionType.valueOf(upperCaseFunctionName);
     } catch (Exception e) {
-      if (FunctionRegistry.getFunctionByName(functionName) != null) {
+      if (FunctionRegistry.containsFunction(functionName)) {
         return SCALAR;
       }
       // Support function name of both jsonExtractScalar and json_extract_scalar
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
index 375ca2b..511dcc8 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
@@ -21,15 +21,12 @@ package org.apache.pinot.common.function.scalar;
 import java.util.concurrent.TimeUnit;
 import org.apache.pinot.common.function.DateTimePatternHandler;
 import org.apache.pinot.common.function.annotations.ScalarFunction;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 
 
 /**
  * Inbuilt date time related transform functions
- * TODO: Exhaustively add all time conversion functions
- *  eg:
- *   1) round(time, roundingValue) - round(minutes, 10), round(millis, 15:MINUTES)
- *   2) simple date time transformations
- *   3) convert(from_format, to_format, bucketing)
  *
  *   NOTE:
  *   <code>toEpochXXXBucket</code> methods are only needed to convert from TimeFieldSpec to DateTimeFieldSpec, to maintain the backward compatibility.
@@ -260,4 +257,299 @@ public class DateTimeFunctions {
   public static long now() {
     return System.currentTimeMillis();
   }
+
+  /**
+   * The {@code timezoneId} for the following methods must be of a Joda-Time format:
+   * https://www.joda.org/joda-time/timezones.html
+   */
+
+  /**
+   * Returns the hour of the time zone offset.
+   */
+  @ScalarFunction
+  public static int timezoneHour(String timezoneId) {
+    return new DateTime(DateTimeZone.forID(timezoneId).getOffset(null), DateTimeZone.UTC).getHourOfDay();
+  }
+
+  /**
+   * Returns the minute of the time zone offset.
+   */
+  @ScalarFunction
+  public static int timezoneMinute(String timezoneId) {
+    return new DateTime(DateTimeZone.forID(timezoneId).getOffset(null), DateTimeZone.UTC).getMinuteOfHour();
+  }
+
+  /**
+   * Returns the year from the given epoch millis in UTC timezone.
+   */
+  @ScalarFunction
+  public static int year(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getYear();
+  }
+
+  /**
+   * Returns the year from the given epoch millis and timezone id.
+   */
+  @ScalarFunction
+  public static int year(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getYear();
+  }
+
+  /**
+   * Returns the year of the ISO week from the given epoch millis in UTC timezone.
+   */
+  @ScalarFunction
+  public static int yearOfWeek(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getWeekyear();
+  }
+
+  /**
+   * Returns the year of the ISO week from the given epoch millis and timezone id.
+   */
+  @ScalarFunction
+  public static int yearOfWeek(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getWeekyear();
+  }
+
+  /**
+   * An alias for yearOfWeek().
+   */
+  @ScalarFunction
+  public static int yow(long millis) {
+    return yearOfWeek(millis);
+  }
+
+  /**
+   * An alias for yearOfWeek().
+   */
+  @ScalarFunction
+  public static int yow(long millis, String timezoneId) {
+    return yearOfWeek(millis, timezoneId);
+  }
+
+  /**
+   * Returns the quarter of the year from the given epoch millis in UTC timezone. The value ranges from 1 to 4.
+   */
+  @ScalarFunction
+  public static int quarter(long millis) {
+    return (month(millis) - 1) / 3 + 1;
+  }
+
+  /**
+   * Returns the quarter of the year from the given epoch millis and timezone id. The value ranges from 1 to 4.
+   */
+  @ScalarFunction
+  public static int quarter(long millis, String timezoneId) {
+    return (month(millis, timezoneId) - 1) / 3 + 1;
+  }
+
+  /**
+   * Returns the month of the year from the given epoch millis in UTC timezone. The value ranges from 1 to 12.
+   */
+  @ScalarFunction
+  public static int month(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getMonthOfYear();
+  }
+
+  /**
+   * Returns the month of the year from the given epoch millis and timezone id. The value ranges from 1 to 12.
+   */
+  @ScalarFunction
+  public static int month(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getMonthOfYear();
+  }
+
+  /**
+   * Returns the ISO week of the year from the given epoch millis in UTC timezone.The value ranges from 1 to 53.
+   */
+  @ScalarFunction
+  public static int week(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getWeekOfWeekyear();
+  }
+
+  /**
+   * Returns the ISO week of the year from the given epoch millis and timezone id. The value ranges from 1 to 53.
+   */
+  @ScalarFunction
+  public static int week(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getWeekOfWeekyear();
+  }
+
+  /**
+   * An alias for week().
+   */
+  @ScalarFunction
+  public static int weekOfYear(long millis) {
+    return week(millis);
+  }
+
+  /**
+   * An alias for week().
+   */
+  @ScalarFunction
+  public static int weekOfYear(long millis, String timezoneId) {
+    return week(millis, timezoneId);
+  }
+
+  /**
+   * Returns the day of the year from the given epoch millis in UTC timezone. The value ranges from 1 to 366.
+   */
+  @ScalarFunction
+  public static int dayOfYear(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getDayOfYear();
+  }
+
+  /**
+   * Returns the day of the year from the given epoch millis and timezone id. The value ranges from 1 to 366.
+   */
+  @ScalarFunction
+  public static int dayOfYear(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getDayOfYear();
+  }
+
+  /**
+   * An alias for dayOfYear().
+   */
+  @ScalarFunction
+  public static int doy(long millis) {
+    return dayOfYear(millis);
+  }
+
+  /**
+   * An alias for dayOfYear().
+   */
+  @ScalarFunction
+  public static int doy(long millis, String timezoneId) {
+    return dayOfYear(millis, timezoneId);
+  }
+
+  /**
+   * Returns the day of the month from the given epoch millis in UTC timezone. The value ranges from 1 to 31.
+   */
+  @ScalarFunction
+  public static int day(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getDayOfMonth();
+  }
+
+  /**
+   * Returns the day of the month from the given epoch millis and timezone id. The value ranges from 1 to 31.
+   */
+  @ScalarFunction
+  public static int day(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getDayOfMonth();
+  }
+
+  /**
+   * An alias for day().
+   */
+  @ScalarFunction
+  public static int dayOfMonth(long millis) {
+    return day(millis);
+  }
+
+  /**
+   * An alias for day().
+   */
+  @ScalarFunction
+  public static int dayOfMonth(long millis, String timezoneId) {
+    return day(millis, timezoneId);
+  }
+
+  /**
+   * Returns the day of the week from the given epoch millis in UTC timezone. The value ranges from 1 (Monday) to 7
+   * (Sunday).
+   */
+  @ScalarFunction
+  public static int dayOfWeek(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getDayOfWeek();
+  }
+
+  /**
+   * Returns the day of the week from the given epoch millis and timezone id. The value ranges from 1 (Monday) to 7
+   * (Sunday).
+   */
+  @ScalarFunction
+  public static int dayOfWeek(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getDayOfWeek();
+  }
+
+  /**
+   * An alias for dayOfWeek().
+   */
+  @ScalarFunction
+  public static int dow(long millis) {
+    return dayOfWeek(millis);
+  }
+
+  /**
+   * An alias for dayOfWeek().
+   */
+  @ScalarFunction
+  public static int dow(long millis, String timezoneId) {
+    return dayOfWeek(millis, timezoneId);
+  }
+
+  /**
+   * Returns the hour of the day from the given epoch millis in UTC timezone. The value ranges from 0 to 23.
+   */
+  @ScalarFunction
+  public static int hour(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getHourOfDay();
+  }
+
+  /**
+   * Returns the hour of the day from the given epoch millis and timezone id. The value ranges from 0 to 23.
+   */
+  @ScalarFunction
+  public static int hour(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getHourOfDay();
+  }
+
+  /**
+   * Returns the minute of the hour from the given epoch millis in UTC timezone. The value ranges from 0 to 59.
+   */
+  @ScalarFunction
+  public static int minute(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getMinuteOfHour();
+  }
+
+  /**
+   * Returns the minute of the hour from the given epoch millis and timezone id. The value ranges from 0 to 59.
+   */
+  @ScalarFunction
+  public static int minute(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getMinuteOfHour();
+  }
+
+  /**
+   * Returns the second of the minute from the given epoch millis in UTC timezone. The value ranges from 0 to 59.
+   */
+  @ScalarFunction
+  public static int second(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getSecondOfMinute();
+  }
+
+  /**
+   * Returns the second of the minute from the given epoch millis and timezone id. The value ranges from 0 to 59.
+   */
+  @ScalarFunction
+  public static int second(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getSecondOfMinute();
+  }
+
+  /**
+   * Returns the millisecond of the second from the given epoch millis in UTC timezone. The value ranges from 0 to 999.
+   */
+  @ScalarFunction
+  public static int millisecond(long millis) {
+    return new DateTime(millis, DateTimeZone.UTC).getMillisOfSecond();
+  }
+
+  /**
+   * Returns the millisecond of the second from the given epoch millis and timezone id. The value ranges from 0 to 999.
+   */
+  @ScalarFunction
+  public static int millisecond(long millis, String timezoneId) {
+    return new DateTime(millis, DateTimeZone.forID(timezoneId)).getMillisOfSecond();
+  }
 }
diff --git a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index 80ceae2..883834e 100644
--- a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++ b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -29,6 +29,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 import org.apache.calcite.config.Lex;
 import org.apache.calcite.sql.SqlBasicCall;
 import org.apache.calcite.sql.SqlDataTypeSpec;
@@ -825,26 +826,27 @@ public class CalciteSqlParser {
     return andExpression;
   }
 
-  protected static Expression invokeCompileTimeFunctionExpression(Expression funcExpr) {
-    if (funcExpr == null || funcExpr.getFunctionCall() == null) {
-      return funcExpr;
+  protected static Expression invokeCompileTimeFunctionExpression(@Nullable Expression expression) {
+    if (expression == null || expression.getFunctionCall() == null) {
+      return expression;
     }
-    Function function = funcExpr.getFunctionCall();
-    int functionOperandsLength = function.getOperandsSize();
+    Function function = expression.getFunctionCall();
+    List<Expression> operands = function.getOperands();
+    int numOperands = operands.size();
     boolean compilable = true;
-    for (int i = 0; i < functionOperandsLength; i++) {
-      Expression operand = invokeCompileTimeFunctionExpression(function.getOperands().get(i));
+    for (int i = 0; i < numOperands; i++) {
+      Expression operand = invokeCompileTimeFunctionExpression(operands.get(i));
       if (operand.getLiteral() == null) {
         compilable = false;
       }
-      function.getOperands().set(i, operand);
+      operands.set(i, operand);
     }
-    String funcName = function.getOperator();
+    String functionName = function.getOperator();
     if (compilable) {
-      FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(funcName);
+      FunctionInfo functionInfo = FunctionRegistry.getFunctionInfo(functionName, numOperands);
       if (functionInfo != null) {
-        Object[] arguments = new Object[functionOperandsLength];
-        for (int i = 0; i < functionOperandsLength; i++) {
+        Object[] arguments = new Object[numOperands];
+        for (int i = 0; i < numOperands; i++) {
           arguments[i] = function.getOperands().get(i).getLiteral().getFieldValue();
         }
         try {
@@ -853,11 +855,13 @@ public class CalciteSqlParser {
           Object result = invoker.invoke(arguments);
           return RequestUtils.getLiteralExpression(result);
         } catch (Exception e) {
-          throw new SqlCompilationException(new IllegalArgumentException("Unsupported function - " + funcName, e));
+          throw new SqlCompilationException(new RuntimeException(
+              "Caught exception while invoking method: " + functionInfo.getMethod() + " with arguments: " + Arrays
+                  .toString(arguments), e));
         }
       }
     }
-    return funcExpr;
+    return expression;
   }
 
   public static boolean isLiteralOnlyExpression(Expression e) {
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluator.java b/pinot-core/src/main/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluator.java
index bacc800..a715b10 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluator.java
@@ -87,7 +87,10 @@ public class InbuiltFunctionEvaluator implements FunctionEvaluator {
       childNodes[i] = childNode;
     }
 
-    FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(function.getFunctionName());
+    FunctionInfo functionInfo = FunctionRegistry.getFunctionInfo(function.getFunctionName(), numArguments);
+    Preconditions
+        .checkState(functionInfo != null, "Unsupported function: %s with %s parameters", function.getFunctionName(),
+            numArguments);
     return new FunctionExecutionNode(functionInfo, childNodes);
   }
 
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
index e307a4b..2e89188 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TransformFunctionFactory.java
@@ -161,6 +161,9 @@ public class TransformFunctionFactory {
       case FUNCTION:
         FunctionContext function = expression.getFunction();
         String functionName = function.getFunctionName();
+        List<ExpressionContext> arguments = function.getArguments();
+        int numArguments = arguments.size();
+
         TransformFunction transformFunction;
         Class<? extends TransformFunction> transformFunctionClass = TRANSFORM_FUNCTION_MAP.get(functionName);
         if (transformFunctionClass != null) {
@@ -172,14 +175,15 @@ public class TransformFunctionFactory {
           }
         } else {
           // Scalar function
-          FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(functionName);
+          FunctionInfo functionInfo = FunctionRegistry.getFunctionInfo(functionName, numArguments);
           if (functionInfo == null) {
-            throw new BadQueryRequestException("Unsupported transform function: " + functionName);
+            throw new BadQueryRequestException(
+                String.format("Unsupported function: %s with %d parameters", functionName, numArguments));
           }
           transformFunction = new ScalarTransformFunctionWrapper(functionInfo);
         }
-        List<ExpressionContext> arguments = function.getArguments();
-        List<TransformFunction> transformFunctionArguments = new ArrayList<>(arguments.size());
+
+        List<TransformFunction> transformFunctionArguments = new ArrayList<>(numArguments);
         for (ExpressionContext argument : arguments) {
           transformFunctionArguments.add(TransformFunctionFactory.get(argument, dataSourceMap));
         }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunction.java
index 9e6d46a..0a046aa 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunction.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunction.java
@@ -36,11 +36,12 @@ public class PostAggregationFunction {
   private final ColumnDataType _resultType;
 
   public PostAggregationFunction(String functionName, ColumnDataType[] argumentTypes) {
-    FunctionInfo functionInfo = FunctionRegistry.getFunctionByName(functionName);
-    Preconditions.checkArgument(functionInfo != null, "Unsupported function: %s", functionName);
+    int numArguments = argumentTypes.length;
+    FunctionInfo functionInfo = FunctionRegistry.getFunctionInfo(functionName, numArguments);
+    Preconditions
+        .checkArgument(functionInfo != null, "Unsupported function: %s with %s parameters", functionName, numArguments);
     _functionInvoker = new FunctionInvoker(functionInfo);
     PinotDataType[] parameterTypes = _functionInvoker.getParameterTypes();
-    int numArguments = argumentTypes.length;
     Preconditions.checkArgument(numArguments == parameterTypes.length,
         "Wrong number of arguments for method: %s, expected: %s, actual: %s", functionInfo.getMethod(),
         parameterTypes.length, numArguments);
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
index e5f34b4..c25b35b 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionEvaluatorTest.java
@@ -18,68 +18,50 @@
  */
 package org.apache.pinot.core.data.function;
 
-import com.google.common.collect.Lists;
+import java.util.Collections;
 import org.apache.pinot.common.function.FunctionRegistry;
 import org.apache.pinot.spi.data.readers.GenericRow;
-import org.joda.time.DateTime;
-import org.joda.time.Days;
-import org.joda.time.MutableDateTime;
-import org.joda.time.format.DateTimeFormat;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
 
 public class InbuiltFunctionEvaluatorTest {
 
   @Test
-  public void testExpressionWithColumn()
-      throws Exception {
-    MyFunc myFunc = new MyFunc();
-    FunctionRegistry.registerFunction(myFunc.getClass().getDeclaredMethod("reverseString", String.class));
-    String expression = "reverseString(testColumn)";
+  public void testExpressionWithColumn() {
+    String expression = "reverse(testColumn)";
     InbuiltFunctionEvaluator evaluator = new InbuiltFunctionEvaluator(expression);
-    Assert.assertEquals(evaluator.getArguments(), Lists.newArrayList("testColumn"));
+    assertEquals(evaluator.getArguments(), Collections.singletonList("testColumn"));
     GenericRow row = new GenericRow();
     for (int i = 0; i < 5; i++) {
       String value = "testValue" + i;
-      row.putField("testColumn", value);
-      Object result = evaluator.evaluate(row);
-      Assert.assertEquals(result, new StringBuilder(value).reverse().toString());
+      row.putValue("testColumn", value);
+      assertEquals(evaluator.evaluate(row), new StringBuilder(value).reverse().toString());
     }
   }
 
   @Test
-  public void testExpressionWithConstant()
-      throws Exception {
-    MyFunc myFunc = new MyFunc();
-    FunctionRegistry
-        .registerFunction(myFunc.getClass().getDeclaredMethod("daysSinceEpoch", String.class, String.class));
-    String input = "1980-01-01";
-    String format = "yyyy-MM-dd";
-    String expression = String.format("daysSinceEpoch('%s', '%s')", input, format);
+  public void testExpressionWithConstant() {
+    String expression = "reverse(12345)";
     InbuiltFunctionEvaluator evaluator = new InbuiltFunctionEvaluator(expression);
-    Assert.assertTrue(evaluator.getArguments().isEmpty());
+    assertTrue(evaluator.getArguments().isEmpty());
     GenericRow row = new GenericRow();
-    Object result = evaluator.evaluate(row);
-    Assert.assertEquals(result, myFunc.daysSinceEpoch(input, format));
+    assertEquals(evaluator.evaluate(row), "54321");
   }
 
   @Test
-  public void testMultiFunctionExpression()
-      throws Exception {
-    MyFunc myFunc = new MyFunc();
-    FunctionRegistry.registerFunction(myFunc.getClass().getDeclaredMethod("reverseString", String.class));
-    FunctionRegistry
-        .registerFunction(myFunc.getClass().getDeclaredMethod("daysSinceEpoch", String.class, String.class));
-    String input = "1980-01-01";
-    String reversedInput = myFunc.reverseString(input);
-    String format = "yyyy-MM-dd";
-    String expression = String.format("daysSinceEpoch(reverseString('%s'), '%s')", reversedInput, format);
+  public void testMultiFunctionExpression() {
+    String expression = "reverse(reverse(testColumn))";
     InbuiltFunctionEvaluator evaluator = new InbuiltFunctionEvaluator(expression);
-    Assert.assertTrue(evaluator.getArguments().isEmpty());
+    assertEquals(evaluator.getArguments(), Collections.singletonList("testColumn"));
     GenericRow row = new GenericRow();
-    Object result = evaluator.evaluate(row);
-    Assert.assertEquals(result, myFunc.daysSinceEpoch(input, format));
+    for (int i = 0; i < 5; i++) {
+      String value = "testValue" + i;
+      row.putValue("testColumn", value);
+      assertEquals(evaluator.evaluate(row), value);
+    }
   }
 
   @Test
@@ -87,36 +69,21 @@ public class InbuiltFunctionEvaluatorTest {
       throws Exception {
     MyFunc myFunc = new MyFunc();
     FunctionRegistry.registerFunction(myFunc.getClass().getDeclaredMethod("appendToStringAndReturn", String.class));
-    String expression = String.format("appendToStringAndReturn('%s')", "test ");
+    String expression = "appendToStringAndReturn('test ')";
     InbuiltFunctionEvaluator evaluator = new InbuiltFunctionEvaluator(expression);
-    Assert.assertTrue(evaluator.getArguments().isEmpty());
+    assertTrue(evaluator.getArguments().isEmpty());
     GenericRow row = new GenericRow();
-    Assert.assertEquals(evaluator.evaluate(row), "test ");
-    Assert.assertEquals(evaluator.evaluate(row), "test test ");
-    Assert.assertEquals(evaluator.evaluate(row), "test test test ");
+    assertEquals(evaluator.evaluate(row), "test ");
+    assertEquals(evaluator.evaluate(row), "test test ");
+    assertEquals(evaluator.evaluate(row), "test test test ");
   }
 
   private static class MyFunc {
-    String reverseString(String input) {
-      return new StringBuilder(input).reverse().toString();
-    }
-
-    MutableDateTime EPOCH_START = new MutableDateTime();
-
-    public MyFunc() {
-      EPOCH_START.setDate(0L);
-    }
-
-    int daysSinceEpoch(String input, String format) {
-      DateTime dateTime = DateTimeFormat.forPattern(format).parseDateTime(input);
-      return Days.daysBetween(EPOCH_START, dateTime).getDays();
-    }
-
-    private String baseString = "";
+    String _baseString = "";
 
     String appendToStringAndReturn(String addedString) {
-      baseString += addedString;
-      return baseString;
+      _baseString += addedString;
+      return _baseString;
     }
   }
 }
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionsTest.java
index df122c8..8c97ead 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionsTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/data/function/InbuiltFunctionsTest.java
@@ -21,6 +21,8 @@ package org.apache.pinot.core.data.function;
 import com.google.common.collect.Lists;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import org.apache.pinot.spi.data.readers.GenericRow;
@@ -216,6 +218,81 @@ public class InbuiltFunctionsTest {
     inputs.add(new Object[]{"fromDateTime(dateTime, \"EEE MMM dd HH:mm:ss ZZZ yyyy\")", Lists.newArrayList(
         "dateTime"), row11_2, 1251142606000L});
 
+    // timezone_hour and timezone_minute
+    List<String> expectedArguments = Collections.singletonList("tz");
+    GenericRow row12_0 = new GenericRow();
+    row12_0.putValue("tz", "UTC");
+    inputs.add(new Object[]{"timezone_hour(tz)", expectedArguments, row12_0, 0});
+    inputs.add(new Object[]{"timezone_minute(tz)", expectedArguments, row12_0, 0});
+
+    GenericRow row12_1 = new GenericRow();
+    row12_1.putValue("tz", "America/Los_Angeles");
+    inputs.add(new Object[]{"timezone_hour(tz)", expectedArguments, row12_1, 17});
+    inputs.add(new Object[]{"timezone_minute(tz)", expectedArguments, row12_1, 0});
+
+    GenericRow row12_2 = new GenericRow();
+    row12_2.putValue("tz", "Pacific/Marquesas");
+    inputs.add(new Object[]{"timezone_hour(tz)", expectedArguments, row12_2, 14});
+    inputs.add(new Object[]{"timezone_minute(tz)", expectedArguments, row12_2, 30});
+
+    GenericRow row12_3 = new GenericRow();
+    row12_3.putValue("tz", "Etc/GMT+12");
+    inputs.add(new Object[]{"timezone_hour(tz)", expectedArguments, row12_3, 12});
+    inputs.add(new Object[]{"timezone_minute(tz)", expectedArguments, row12_3, 0});
+
+    GenericRow row12_4 = new GenericRow();
+    row12_4.putValue("tz", "Etc/GMT+1");
+    inputs.add(new Object[]{"timezone_hour(tz)", expectedArguments, row12_4, 23});
+    inputs.add(new Object[]{"timezone_minute(tz)", expectedArguments, row12_4, 0});
+
+    // Convenience extraction functions
+    expectedArguments = Collections.singletonList("millis");
+    GenericRow row13_0 = new GenericRow();
+    // Sat May 23 2020 22:23:13.123 UTC
+    row13_0.putValue("millis", 1590272593123L);
+
+    inputs.add(new Object[]{"year(millis)", expectedArguments, row13_0, 2020});
+    inputs.add(new Object[]{"year_of_week(millis)", expectedArguments, row13_0, 2020});
+    inputs.add(new Object[]{"yow(millis)", expectedArguments, row13_0, 2020});
+    inputs.add(new Object[]{"quarter(millis)", expectedArguments, row13_0, 2});
+    inputs.add(new Object[]{"month(millis)", expectedArguments, row13_0, 5});
+    inputs.add(new Object[]{"week(millis)", expectedArguments, row13_0, 21});
+    inputs.add(new Object[]{"week_of_year(millis)", expectedArguments, row13_0, 21});
+    inputs.add(new Object[]{"day_of_year(millis)", expectedArguments, row13_0, 144});
+    inputs.add(new Object[]{"doy(millis)", expectedArguments, row13_0, 144});
+    inputs.add(new Object[]{"day(millis)", expectedArguments, row13_0, 23});
+    inputs.add(new Object[]{"day_of_month(millis)", expectedArguments, row13_0, 23});
+    inputs.add(new Object[]{"day_of_week(millis)", expectedArguments, row13_0, 6});
+    inputs.add(new Object[]{"dow(millis)", expectedArguments, row13_0, 6});
+    inputs.add(new Object[]{"hour(millis)", expectedArguments, row13_0, 22});
+    inputs.add(new Object[]{"minute(millis)", expectedArguments, row13_0, 23});
+    inputs.add(new Object[]{"second(millis)", expectedArguments, row13_0, 13});
+    inputs.add(new Object[]{"millisecond(millis)", expectedArguments, row13_0, 123});
+
+    expectedArguments = Arrays.asList("millis", "tz");
+    GenericRow row13_1 = new GenericRow();
+    // Sat May 23 2020 15:23:13.123 America/Los_Angeles
+    row13_1.putValue("millis", 1590272593123L);
+    row13_1.putValue("tz", "America/Los_Angeles");
+
+    inputs.add(new Object[]{"year(millis, tz)", expectedArguments, row13_1, 2020});
+    inputs.add(new Object[]{"year_of_week(millis, tz)", expectedArguments, row13_1, 2020});
+    inputs.add(new Object[]{"yow(millis, tz)", expectedArguments, row13_1, 2020});
+    inputs.add(new Object[]{"quarter(millis, tz)", expectedArguments, row13_1, 2});
+    inputs.add(new Object[]{"month(millis, tz)", expectedArguments, row13_1, 5});
+    inputs.add(new Object[]{"week(millis, tz)", expectedArguments, row13_1, 21});
+    inputs.add(new Object[]{"week_of_year(millis, tz)", expectedArguments, row13_1, 21});
+    inputs.add(new Object[]{"day_of_year(millis, tz)", expectedArguments, row13_1, 144});
+    inputs.add(new Object[]{"doy(millis, tz)", expectedArguments, row13_1, 144});
+    inputs.add(new Object[]{"day(millis, tz)", expectedArguments, row13_1, 23});
+    inputs.add(new Object[]{"day_of_month(millis, tz)", expectedArguments, row13_1, 23});
+    inputs.add(new Object[]{"day_of_week(millis, tz)", expectedArguments, row13_1, 6});
+    inputs.add(new Object[]{"dow(millis, tz)", expectedArguments, row13_1, 6});
+    inputs.add(new Object[]{"hour(millis, tz)", expectedArguments, row13_1, 15});
+    inputs.add(new Object[]{"minute(millis, tz)", expectedArguments, row13_1, 23});
+    inputs.add(new Object[]{"second(millis, tz)", expectedArguments, row13_1, 13});
+    inputs.add(new Object[]{"millisecond(millis, tz)", expectedArguments, row13_1, 123});
+
     return inputs.toArray(new Object[0][]);
   }
 


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