richardstartin commented on a change in pull request #8253:
URL: https://github.com/apache/pinot/pull/8253#discussion_r816133799
##########
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:
Sorry, allow me to clarify:
```java
Set<String> set = new HashSet<>();
Method[] methods = StringFunctions.class.getDeclaredMethods();
for (Method method : methods) {
if (method.getReturnType() == String.class) {
if (method.isAnnotationPresent(ScalarFunction.class)) {
ScalarFunction annotation =
method.getAnnotation(ScalarFunction.class);
for (String name : annotation.names()) {
set.add(name.toUpperCase());
}
}
set.add(method.getName().toUpperCase());
}
}
return set;
```
This way your logic works even if there is an alias in use. Does that make
sense?
--
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
