Jackie-Jiang commented on a change in pull request #8137:
URL: https://github.com/apache/pinot/pull/8137#discussion_r801898017
##########
File path:
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +88,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
Review comment:
We can directly return `false` here?
##########
File path:
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
##########
@@ -81,14 +87,28 @@ public String getName() {
return _name;
}
+ public static boolean isAggregationFunction(String functionName) {
+ if (NAMES.contains(functionName)) {
+ return true;
+ }
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
+ try {
+ getAggregationFunctionType(functionName);
+ return true;
+ } catch (Exception ignore) {
+ }
+ }
+ String upperCaseFunctionName = functionName.replace("_", "").toUpperCase();
+ return NAMES.contains(upperCaseFunctionName);
+ }
+
/**
* Returns the corresponding aggregation function type for the given
function name.
* <p>NOTE: Underscores in the function name are ignored.
*/
public static AggregationFunctionType getAggregationFunctionType(String
functionName) {
- String upperCaseFunctionName = StringUtils.remove(functionName,
'_').toUpperCase();
- if (upperCaseFunctionName.startsWith("PERCENTILE")) {
- String remainingFunctionName = upperCaseFunctionName.substring(10);
+ if (functionName.regionMatches(true, 0, "percentile", 0, 10)) {
Review comment:
I believe the allocation is from the `StringUtils.remove(functionName,
'_').toUpperCase()` which cannot be avoided in most of cases (non-percentile
function).
A little behavior change here if we use `regionMatches` is that
`percent_ile` cannot be recognized (not really important though).
Either way is fine, you can make the decision
--
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
