LadyForest commented on code in PR #23478:
URL: https://github.com/apache/flink/pull/23478#discussion_r1368880765
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/ScalarOperatorGens.scala:
##########
@@ -345,44 +345,86 @@ object ScalarOperatorGens {
}
}
- def generateEquals(
+ private def wrapExpressionIfNonEq(
+ isNonEq: Boolean,
+ equalsExpr: GeneratedExpression,
+ resultType: LogicalType): GeneratedExpression = {
+ if (isNonEq) {
+ GeneratedExpression(
+ s"(!${equalsExpr.resultTerm})",
+ equalsExpr.nullTerm,
+ equalsExpr.code,
+ resultType)
+ } else {
+ equalsExpr
+ }
+ }
+
+ private def generateEqualAndNonEqual(
ctx: CodeGeneratorContext,
left: GeneratedExpression,
right: GeneratedExpression,
+ operator: String,
resultType: LogicalType): GeneratedExpression = {
+
checkImplicitConversionValidity(left, right)
+
+ val nonEq = operator match {
+ case "==" => false
+ case "!=" => true
+ case _ => throw new CodeGenException(s"Unsupported boolean comparison
'$operator'.")
+ }
val canEqual = isInteroperable(left.resultType, right.resultType)
+
if (isCharacterString(left.resultType) &&
isCharacterString(right.resultType)) {
- generateOperatorIfNotNull(ctx, resultType, left, right)(
- (leftTerm, rightTerm) => s"$leftTerm.equals($rightTerm)")
+ wrapExpressionIfNonEq(
Review Comment:
Hi @fengjiajie, I have reconsidered and realized that the equal and
non-equal comparisons of string types cannot directly invoke the
"wrapExpressionIfNonEq" function. For example, in TPC-DS Q19, the generated
join operator's non-equal conditions are as follows:
```java
// right, generated by
// generateOperatorIfNotNull(ctx, resultType, left, right)((leftTerm,
rightTerm) => s"$leftTerm.equals($rightTerm)")
isNull$854 = isNull$851 || false || false;
result$855 = org.apache.flink.table.data.binary.BinaryStringData.EMPTY_UTF8;
if (!isNull$854) {
result$855 =
org.apache.flink.table.data.binary.BinaryStringDataUtil.substringSQL(field$853,
((int) 1), ((int) 5));
isNull$854 = (result$855 == null);
}
isNull$858 = isNull$856 || false || false;
result$859 = org.apache.flink.table.data.binary.BinaryStringData.EMPTY_UTF8;
if (!isNull$858) {
result$859 =
org.apache.flink.table.data.binary.BinaryStringDataUtil.substringSQL(field$857,
((int) 1), ((int) 5));
isNull$858 = (result$859 == null);
}
isNull$860 = isNull$854 || isNull$858;
result$861 = false;
if (!isNull$860) {
result$861 = !result$855.equals(result$859);
}
return result$861;
```
```java
// wrong, generated by wrapExpressionIfNonEq(...)
isNull$854 = isNull$851 || false || false;
result$855 = org.apache.flink.table.data.binary.BinaryStringData.EMPTY_UTF8;
if (!isNull$854) {
result$855 =
org.apache.flink.table.data.binary.BinaryStringDataUtil.substringSQL(field$853,
((int) 1), ((int) 5));
isNull$854 = (result$855 == null);
}
isNull$858 = isNull$856 || false || false;
result$859 = org.apache.flink.table.data.binary.BinaryStringData.EMPTY_UTF8;
if (!isNull$858) {
result$859 =
org.apache.flink.table.data.binary.BinaryStringDataUtil.substringSQL(field$857,
((int) 1), ((int) 5));
isNull$858 = (result$859 == null);
}
isNull$860 = isNull$854 || isNull$858;
result$861 = false;
if (!isNull$860) {
result$861 = result$855.equals(result$859);
}
return (!result$861); // this is wrong
```
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
