Jackie-Jiang commented on a change in pull request #8304:
URL: https://github.com/apache/pinot/pull/8304#discussion_r822011259
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -104,4 +111,30 @@ public static double ln(double a) {
public static double sqrt(double a) {
return Math.sqrt(a);
}
+
+ @ScalarFunction
+ public static double sign(double a) {
+ return Math.signum(a);
+ }
+
+ @ScalarFunction
+ public static double pi() { return Math.PI; }
+
+ @ScalarFunction
+ public static double e() { return Math.E; }
+
+ @ScalarFunction
+ public static double power(double a, double b) {
+ return Math.pow(a, b);
+ }
+
+ @ScalarFunction
+ public static double log(double a) {
+ return Math.log10(a);
+ }
+
+ @ScalarFunction(names = {"round_decimal"})
Review comment:
No need to use this alias. Both `round_decimal` and `roundDecimal` will
be canonicalized to `rounddecimal`
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -180,6 +180,24 @@ public static String rtrim(String input) {
return RTRIM.matcher(input).replaceAll("");
}
+ /**
+ * @param input
+ * @return trim spaces from right side of the string
+ */
+ @ScalarFunction
+ public static String leftSubStr(String input, int length) {
+ return StringUtils.left(input, length);
+ }
+
+ /**
+ * @param input
+ * @return trim spaces from right side of the string
Review comment:
Fix the javadoc
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -715,4 +730,41 @@ public static String dateTimeConvert(String timeValueStr,
String inputFormatStr,
return new
DateTimeFormatSpec(outputFormatStr).fromMillisToFormat(roundedTimeValueMs);
}
}
+
+ @ScalarFunction(names = {"timestampAdd", "dateAdd"})
Review comment:
Please add some javadoc for these 2 methods
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -392,6 +431,45 @@ public static String normalize(String input, String form) {
return StringUtils.split(input, delimiter);
}
+ /**
+ * see String#split(String)
+ * @param input
+ * @param delimiter
+ * @return splits string on specified delimiter and returns an array.
+ */
+ @ScalarFunction
+ public static String split(String input, String delimiter, int index) {
+ String[] splitString = StringUtils.split(input, delimiter);
+ if(index < splitString.length){
+ return splitString[index];
+ } else {
+ return "null";
+ }
+ }
+
+ /**
+ * see String#split(String)
+ * @param input
+ * @param times
+ * @return splits string on specified delimiter and returns an array.
+ */
+ @ScalarFunction
+ public static String repeat(String input, int times) {
+ return StringUtils.repeat(input, times);
+ }
+
+ /**
+ * see String#split(String)
Review comment:
Update the javadoc
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -392,6 +431,45 @@ public static String normalize(String input, String form) {
return StringUtils.split(input, delimiter);
}
+ /**
+ * see String#split(String)
+ * @param input
+ * @param delimiter
+ * @return splits string on specified delimiter and returns an array.
+ */
+ @ScalarFunction
+ public static String split(String input, String delimiter, int index) {
Review comment:
This is `splitPart` in presto (index starts with 1 though):
https://prestodb.io/docs/current/functions/string.html
Consider making them consistent
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -104,4 +111,30 @@ public static double ln(double a) {
public static double sqrt(double a) {
return Math.sqrt(a);
}
+
+ @ScalarFunction
+ public static double sign(double a) {
+ return Math.signum(a);
+ }
+
+ @ScalarFunction
+ public static double pi() { return Math.PI; }
+
+ @ScalarFunction
+ public static double e() { return Math.E; }
+
+ @ScalarFunction
+ public static double power(double a, double b) {
+ return Math.pow(a, b);
+ }
+
+ @ScalarFunction
+ public static double log(double a) {
Review comment:
We can also add `log2`
```suggestion
public static double log10(double a) {
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -715,4 +730,41 @@ public static String dateTimeConvert(String timeValueStr,
String inputFormatStr,
return new
DateTimeFormatSpec(outputFormatStr).fromMillisToFormat(roundedTimeValueMs);
}
}
+
+ @ScalarFunction(names = {"timestampAdd", "dateAdd"})
+ public static long timestampAdd(String unit, int interval, long timestamp) {
+ ISOChronology chronology = ISOChronology.getInstanceUTC();
+ long millis = getTimestampField(chronology, unit).add(timestamp, interval);
+ return millis;
+ }
+
+ @ScalarFunction(names = {"timestampDiff", "dateDiff"})
+ public static long timestampDiff(String unit, long timestamp1, long
timestamp2) {
+ ISOChronology chronology = ISOChronology.getInstanceUTC();
+ long millis = getTimestampField(chronology,
unit).getDifferenceAsLong(timestamp1, timestamp2);
Review comment:
The return is not `millis` but value in `unit`. Also, seems the argument
is in wrong order
```suggestion
return getTimestampField(chronology,
unit).getDifferenceAsLong(timestamp2, timestamp1);
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -180,6 +180,24 @@ public static String rtrim(String input) {
return RTRIM.matcher(input).replaceAll("");
}
+ /**
+ * @param input
+ * @return trim spaces from right side of the string
Review comment:
Fix the javadoc
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/TrigonometricFunctions.java
##########
@@ -0,0 +1,74 @@
+package org.apache.pinot.common.function.scalar;
+
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class TrigonometricFunctions {
+ private TrigonometricFunctions(){
+ }
+
+ @ScalarFunction
+ public static double sin(double a) {
+ return Math.sin(a);
+ }
+
+ @ScalarFunction
+ public static double cos(double a) {
+ return Math.cos(a);
+ }
+
+ @ScalarFunction
+ public static double tan(double a) {
+ return Math.tan(a);
+ }
+
+ @ScalarFunction
+ public static double cot(double a) {
+ return Math.sin(a);
+ }
+
+ @ScalarFunction
+ public static double asin(double a) {
+ return Math.asin(a);
+ }
+
+ @ScalarFunction
+ public static double acos(double a) {
+ return Math.acos(a);
+ }
+
+ @ScalarFunction
+ public static double atan(double a) {
+ return Math.atan(a);
+ }
+
+ @ScalarFunction
+ public static double atan2(double a, double b) {
+ return Math.atan2(a, b);
+ }
+
+ @ScalarFunction
+ public static double sinh(double a) {
+ return Math.sinh(a);
+ }
+
+ @ScalarFunction
+ public static double cosh(double a) {
+ return Math.cosh(a);
+ }
+
+ @ScalarFunction
+ public static double tanh(double a) {
+ return Math.tanh(a);
+ }
+
+ @ScalarFunction
+ public static double degrees(double a) {
+ return Math.toDegrees(a);
+ }
+
+ @ScalarFunction
+ public static double radian(double a) {
Review comment:
```suggestion
public static double radians(double a) {
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/TrigonometricFunctions.java
##########
@@ -0,0 +1,74 @@
+package org.apache.pinot.common.function.scalar;
+
+import org.apache.pinot.spi.annotations.ScalarFunction;
+
+
+public class TrigonometricFunctions {
+ private TrigonometricFunctions(){
+ }
+
+ @ScalarFunction
+ public static double sin(double a) {
+ return Math.sin(a);
+ }
+
+ @ScalarFunction
+ public static double cos(double a) {
+ return Math.cos(a);
+ }
+
+ @ScalarFunction
+ public static double tan(double a) {
+ return Math.tan(a);
+ }
+
+ @ScalarFunction
+ public static double cot(double a) {
Review comment:
What is `cot`?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -104,4 +111,30 @@ public static double ln(double a) {
public static double sqrt(double a) {
return Math.sqrt(a);
}
+
+ @ScalarFunction
+ public static double sign(double a) {
+ return Math.signum(a);
+ }
+
+ @ScalarFunction
+ public static double pi() { return Math.PI; }
+
+ @ScalarFunction
+ public static double e() { return Math.E; }
+
+ @ScalarFunction
Review comment:
```suggestion
@ScalarFunction(names = {"pow", "power"})
```
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -715,4 +730,41 @@ public static String dateTimeConvert(String timeValueStr,
String inputFormatStr,
return new
DateTimeFormatSpec(outputFormatStr).fromMillisToFormat(roundedTimeValueMs);
}
}
+
+ @ScalarFunction(names = {"timestampAdd", "dateAdd"})
+ public static long timestampAdd(String unit, int interval, long timestamp) {
Review comment:
Should we put `interval` as `long`?
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -273,9 +274,23 @@ public static String toDateTime(long millis, String
pattern, String timezoneId)
*/
@ScalarFunction
public static long fromDateTime(String dateTimeString, String pattern) {
+ if (StringUtils.isEmpty(dateTimeString)) {
Review comment:
Any specific reason why we want to support empty string? IMO it should
throw exception if the time value doesn't comply with the pattern
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -392,6 +431,45 @@ public static String normalize(String input, String form) {
return StringUtils.split(input, delimiter);
}
+ /**
+ * see String#split(String)
Review comment:
Update the javadoc
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/DateTimeFunctions.java
##########
@@ -715,4 +730,41 @@ public static String dateTimeConvert(String timeValueStr,
String inputFormatStr,
return new
DateTimeFormatSpec(outputFormatStr).fromMillisToFormat(roundedTimeValueMs);
}
}
+
+ @ScalarFunction(names = {"timestampAdd", "dateAdd"})
+ public static long timestampAdd(String unit, int interval, long timestamp) {
+ ISOChronology chronology = ISOChronology.getInstanceUTC();
+ long millis = getTimestampField(chronology, unit).add(timestamp, interval);
+ return millis;
+ }
+
+ @ScalarFunction(names = {"timestampDiff", "dateDiff"})
+ public static long timestampDiff(String unit, long timestamp1, long
timestamp2) {
+ ISOChronology chronology = ISOChronology.getInstanceUTC();
+ long millis = getTimestampField(chronology,
unit).getDifferenceAsLong(timestamp1, timestamp2);
+ return millis;
+ }
+
+ private static DateTimeField getTimestampField(ISOChronology chronology,
String unit) {
Review comment:
This is available in `DateTimeUtils`
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
##########
@@ -392,6 +431,45 @@ public static String normalize(String input, String form) {
return StringUtils.split(input, delimiter);
}
+ /**
+ * see String#split(String)
+ * @param input
+ * @param delimiter
+ * @return splits string on specified delimiter and returns an array.
+ */
+ @ScalarFunction
+ public static String split(String input, String delimiter, int index) {
+ String[] splitString = StringUtils.split(input, delimiter);
+ if(index < splitString.length){
+ return splitString[index];
+ } else {
+ return "null";
+ }
+ }
+
+ /**
+ * see String#split(String)
Review comment:
Update the javadoc
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArithmeticFunctions.java
##########
@@ -104,4 +111,30 @@ public static double ln(double a) {
public static double sqrt(double a) {
return Math.sqrt(a);
}
+
+ @ScalarFunction
+ public static double sign(double a) {
+ return Math.signum(a);
+ }
+
+ @ScalarFunction
+ public static double pi() { return Math.PI; }
+
+ @ScalarFunction
+ public static double e() { return Math.E; }
+
+ @ScalarFunction
+ public static double power(double a, double b) {
+ return Math.pow(a, b);
+ }
+
+ @ScalarFunction
+ public static double log(double a) {
+ return Math.log10(a);
+ }
+
+ @ScalarFunction(names = {"round_decimal"})
Review comment:
Can you add a `TODO` stating this should be named as `round` but the
name is taken in the `DateTimeFunctions`
--
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
