richardstartin commented on a change in pull request #8253:
URL: https://github.com/apache/pinot/pull/8253#discussion_r816041704



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/statement/StringPredicateFilterOptimizer.java
##########
@@ -99,21 +104,37 @@ private static void 
replaceMinusWithCompareForStrings(Expression expression, Sch
     Function function = expression.getFunctionCall();
     String operator = function.getOperator();
     List<Expression> operands = function.getOperands();
-    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && 
isStringColumn(operands.get(0), schema)
-        && isStringColumn(operands.get(1), schema)) {
+    if (operator.equals(MINUS_OPERATOR_NAME) && operands.size() == 2 && 
isString(operands.get(0), schema)
+        && isString(operands.get(1), schema)) {
       function.setOperator(STRCMP_OPERATOR_NAME);
     }
   }
 
-  /** @return true if expression is a column of string type. */
-  private static boolean isStringColumn(Expression expression, Schema schema) {
-    if (expression.getType() != ExpressionType.IDENTIFIER) {
-      // Expression is not a column.
-      return false;
+  /** @return true if expression is STRING column or a function that outputs 
STRING. */
+  private static boolean isString(Expression expression, Schema schema) {
+    ExpressionType expressionType = expression.getType();
+
+    if (expressionType == ExpressionType.IDENTIFIER) {
+      // Check if this is a STRING column.
+      String column = expression.getIdentifier().getName();
+      FieldSpec fieldSpec = schema.getFieldSpecFor(column);
+      return fieldSpec != null && fieldSpec.getDataType() == 
FieldSpec.DataType.STRING;
     }
 
-    String column = expression.getIdentifier().getName();
-    FieldSpec fieldSpec = schema.getFieldSpecFor(column);
-    return fieldSpec != null && fieldSpec.getDataType() == 
FieldSpec.DataType.STRING;
+    // Check if the function returns STRING as output.
+    return (expressionType == ExpressionType.FUNCTION && 
STRING_OUTPUT_FUNCTIONS
+        .contains(expression.getFunctionCall().getOperator().toUpperCase()));
+  }
+
+  /** List of string functions that return STRING output. */
+  private static Set<String> getStringOutputFunctionList() {
+    Set<String> set = new HashSet<>();
+    Method[] methods = StringFunctions.class.getDeclaredMethods();
+    for (Method method : methods) {
+      if (method.getReturnType() == String.class) {
+        set.add(method.getName().toUpperCase());
+      }
+    }
+    return set;

Review comment:
       `@ScalarFunction` now has a `names` array parameter - so you need to 
read the names from the annotation to do this correctly.




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