Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix](nereids)NullSafeEqualToEqual rule should keep <=> unchanged if it has none-literal child #36521

Merged
merged 1 commit into from
Jun 20, 2024
Merged

[fix](nereids)NullSafeEqualToEqual rule should keep <=> unchanged if it has none-literal child #36521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...-core/src/main/java/org/apache/doris/nereids/rules/expression/ExpressionOptimization.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.doris.nereids.rules.expression.rules.DistinctPredicatesRule;
import org.apache.doris.nereids.rules.expression.rules.ExtractCommonFactorRule;
import org.apache.doris.nereids.rules.expression.rules.LikeToEqualRewrite;
import org.apache.doris.nereids.rules.expression.rules.NullSafeEqualToEqual;
import org.apache.doris.nereids.rules.expression.rules.OrToIn;
import org.apache.doris.nereids.rules.expression.rules.SimplifyComparisonPredicate;
import org.apache.doris.nereids.rules.expression.rules.SimplifyDecimalV3Comparison;
Expand Down Expand Up @@ -51,6 +52,7 @@ public class ExpressionOptimization extends ExpressionRewrite {
ArrayContainToArrayOverlap.INSTANCE,
CaseWhenToIf.INSTANCE,
TopnToMax.INSTANCE,
NullSafeEqualToEqual.INSTANCE,
LikeToEqualRewrite.INSTANCE
)
);
Expand Down
24 changes: 9 additions & 15 deletions ...e/src/main/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqual.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,16 @@
import org.apache.doris.nereids.trees.expressions.IsNull;
import org.apache.doris.nereids.trees.expressions.NullSafeEqual;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;

import com.google.common.collect.ImmutableList;

import java.util.List;

/**
* convert "<=>" to "=", if both sides are not nullable
* convert "A <=> null" to "A is null"
* null <=> null : true
* null <=> 1 : false
* 1 <=> 2 : 1 = 2
*/
public class NullSafeEqualToEqual implements ExpressionPatternRuleFactory {
public static final NullSafeEqualToEqual INSTANCE = new NullSafeEqualToEqual();
Expand All @@ -47,19 +46,14 @@ public List<ExpressionPatternMatcher<? extends Expression>> buildRules() {
}

private static Expression rewrite(NullSafeEqual nullSafeEqual) {
if (nullSafeEqual.left() instanceof NullLiteral) {
if (nullSafeEqual.right().nullable()) {
return new IsNull(nullSafeEqual.right());
} else {
return BooleanLiteral.FALSE;
}
} else if (nullSafeEqual.right() instanceof NullLiteral) {
if (nullSafeEqual.left().nullable()) {
return new IsNull(nullSafeEqual.left());
} else {
return BooleanLiteral.FALSE;
}
} else if (!nullSafeEqual.left().nullable() && !nullSafeEqual.right().nullable()) {
// because the nullable info hasn't been finalized yet, the optimization is limited
if (nullSafeEqual.left().isNullLiteral() && nullSafeEqual.right().isNullLiteral()) {
return BooleanLiteral.TRUE;
} else if (nullSafeEqual.left().isNullLiteral()) {
return nullSafeEqual.right().isLiteral() ? BooleanLiteral.FALSE : new IsNull(nullSafeEqual.right());
} else if (nullSafeEqual.right().isNullLiteral()) {
return nullSafeEqual.left().isLiteral() ? BooleanLiteral.FALSE : new IsNull(nullSafeEqual.left());
} else if (nullSafeEqual.left().isLiteral() && nullSafeEqual.right().isLiteral()) {
return new EqualTo(nullSafeEqual.left(), nullSafeEqual.right());
}
return nullSafeEqual;
Expand Down
38 changes: 30 additions & 8 deletions ...c/test/java/org/apache/doris/nereids/rules/expression/rules/NullSafeEqualToEqualTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.doris.nereids.trees.expressions.NullSafeEqual;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.literal.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.types.StringType;

Expand All @@ -32,29 +33,39 @@

class NullSafeEqualToEqualTest extends ExpressionRewriteTestHelper {

// "A<=> Null" to "A is null"
// "A <=> Null" to "A is null"
@Test
void testNullSafeEqualToIsNull() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
SlotReference slot = new SlotReference("a", StringType.INSTANCE, true);
assertRewrite(new NullSafeEqual(slot, NullLiteral.INSTANCE), new IsNull(slot));
slot = new SlotReference("a", StringType.INSTANCE, false);
assertRewrite(new NullSafeEqual(slot, NullLiteral.INSTANCE), new IsNull(slot));
}

// "A<=> Null" to "False", when A is not nullable
// "0 <=> Null" to false
@Test
void testNullSafeEqualToFalse() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
SlotReference slot = new SlotReference("a", StringType.INSTANCE, false);
assertRewrite(new NullSafeEqual(slot, NullLiteral.INSTANCE), BooleanLiteral.FALSE);
assertRewrite(new NullSafeEqual(new IntegerLiteral(0), NullLiteral.INSTANCE), BooleanLiteral.FALSE);
}

// "NULL <=> Null" to false
@Test
void testNullSafeEqualToTrue() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
assertRewrite(new NullSafeEqual(NullLiteral.INSTANCE, NullLiteral.INSTANCE), BooleanLiteral.TRUE);
}

// "A(nullable)<=>B" not changed
@Test
void testNullSafeEqualNotChangedLeft() {
void testNullSafeEqualNotChangedLeftNullable() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
Expand All @@ -65,7 +76,7 @@ void testNullSafeEqualNotChangedLeft() {

// "A<=>B(nullable)" not changed
@Test
void testNullSafeEqualNotChangedRight() {
void testNullSafeEqualNotChangedRightNullable() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
Expand All @@ -74,14 +85,25 @@ void testNullSafeEqualNotChangedRight() {
assertRewrite(new NullSafeEqual(a, b), new NullSafeEqual(a, b));
}

// "A<=>B" changed
// "A<=>B" not changed
@Test
void testNullSafeEqualToEqual() {
void testNullSafeEqualNotChangedBothNullable() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
SlotReference a = new SlotReference("a", StringType.INSTANCE, false);
SlotReference b = new SlotReference("b", StringType.INSTANCE, false);
assertRewrite(new NullSafeEqual(a, b), new NullSafeEqual(a, b));
}

// "1 <=> 0" to "1 = 0"
@Test
void testNullSafeEqualToEqual() {
executor = new ExpressionRuleExecutor(ImmutableList.of(
bottomUp(NullSafeEqualToEqual.INSTANCE)
));
IntegerLiteral a = new IntegerLiteral(0);
IntegerLiteral b = new IntegerLiteral(1);
assertRewrite(new NullSafeEqual(a, b), new EqualTo(a, b));
}
}
Loading