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

[ESQL] Remove unused NullEquals class #108022

Merged
merged 2 commits into from
Apr 30, 2024
Merged

[ESQL] Remove unused NullEquals class #108022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0abb49c
Remove NullEquals
not-napoleon Apr 29, 2024
b0c8d78
fix merge conflicts
not-napoleon Apr 30, 2024
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
48 changes: 0 additions & 48 deletions ...ava/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/NullEquals.java
View file
Open in desktop

This file was deleted.

4 changes: 1 addition & 3 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRules.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NullEquals;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.Enrich;
import org.elasticsearch.xpack.esql.plan.logical.EsRelation;
Expand Down Expand Up @@ -333,8 +332,7 @@ private static Expression propagate(And and) {
for (Expression ex : Predicates.splitAnd(and)) {
if (ex instanceof Range) {
ranges.add((Range) ex);
} else if (ex instanceof Equals || ex instanceof NullEquals) {
BinaryComparison otherEq = (BinaryComparison) ex;
} else if (ex instanceof Equals otherEq) {
// equals on different values evaluate to FALSE
// ignore date/time fields as equality comparison might actually be a range check
if (otherEq.right().foldable() && DataTypes.isDateTime(otherEq.left().dataType()) == false) {
Expand Down
6 changes: 2 additions & 4 deletions ...in/esql/src/main/java/org/elasticsearch/xpack/esql/planner/EsqlExpressionTranslators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NullEquals;
import org.elasticsearch.xpack.esql.querydsl.query.SpatialRelatesQuery;
import org.elasticsearch.xpack.ql.QlIllegalArgumentException;
import org.elasticsearch.xpack.ql.expression.Expression;
Expand Down Expand Up @@ -133,7 +132,6 @@ static Query translate(InsensitiveEquals bc) {
* <ul>
* <li>{@link Equals}</li>
* <li>{@link NotEquals}</li>
* <li>{@link NullEquals}</li>
* <li>{@link GreaterThanOrEqual}</li>
* <li>{@link GreaterThan}</li>
* <li>{@link LessThanOrEqual}</li>
Expand Down Expand Up @@ -221,7 +219,7 @@ static Query translate(BinaryComparison bc, TranslatorHandler handler) {
if (bc instanceof LessThanOrEqual) {
return new RangeQuery(source, name, null, false, value, true, format, zoneId);
}
if (bc instanceof Equals || bc instanceof NullEquals || bc instanceof NotEquals) {
if (bc instanceof Equals || bc instanceof NotEquals) {
name = pushableAttributeName(attribute);

Query query;
Expand Down Expand Up @@ -273,7 +271,7 @@ private static Query translateOutOfRangeComparisons(BinaryComparison bc) {
matchAllOrNone = (num.doubleValue() > 0) == false;
} else if (bc instanceof LessThan || bc instanceof LessThanOrEqual) {
matchAllOrNone = (num.doubleValue() > 0);
} else if (bc instanceof Equals || bc instanceof NullEquals) {
} else if (bc instanceof Equals) {
matchAllOrNone = false;
} else if (bc instanceof NotEquals) {
matchAllOrNone = true;
Expand Down
38 changes: 0 additions & 38 deletions ...plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRulesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NullEquals;
import org.elasticsearch.xpack.ql.TestUtils;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.FieldAttribute;
Expand Down Expand Up @@ -65,10 +64,6 @@ public static NotEquals notEqualsOf(Expression left, Expression right) {
return new NotEquals(EMPTY, left, right, randomZone());
}

public static NullEquals nullEqualsOf(Expression left, Expression right) {
return new NullEquals(EMPTY, left, right, randomZone());
}

public static LessThanOrEqual lessThanOrEqualOf(Expression left, Expression right) {
return new LessThanOrEqual(EMPTY, left, right, randomZone());
}
Expand Down Expand Up @@ -227,17 +222,6 @@ public void testDualEqualsConjunction() {
assertEquals(FALSE, exp);
}

// a <=> 1 AND a <=> 2 -> FALSE
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test cases were just copied over from the QL version of the optimizer rules.

public void testDualNullEqualsConjunction() {
FieldAttribute fa = getFieldAttribute();
NullEquals eq1 = nullEqualsOf(fa, ONE);
NullEquals eq2 = nullEqualsOf(fa, TWO);

OptimizerRules.PropagateEquals rule = new OptimizerRules.PropagateEquals();
Expression exp = rule.rule(new And(EMPTY, eq1, eq2));
assertEquals(FALSE, exp);
}

// 1 < a < 10 AND a == 10 -> FALSE
public void testEliminateRangeByEqualsOutsideInterval() {
FieldAttribute fa = getFieldAttribute();
Expand All @@ -249,17 +233,6 @@ public void testEliminateRangeByEqualsOutsideInterval() {
assertEquals(FALSE, exp);
}

// 1 < a < 10 AND a <=> 10 -> FALSE
public void testEliminateRangeByNullEqualsOutsideInterval() {
FieldAttribute fa = getFieldAttribute();
NullEquals eq1 = nullEqualsOf(fa, new Literal(EMPTY, 10, DataTypes.INTEGER));
Range r = rangeOf(fa, ONE, false, new Literal(EMPTY, 10, DataTypes.INTEGER), false);

OptimizerRules.PropagateEquals rule = new OptimizerRules.PropagateEquals();
Expression exp = rule.rule(new And(EMPTY, eq1, r));
assertEquals(FALSE, exp);
}

// a != 3 AND a = 3 -> FALSE
public void testPropagateEquals_VarNeq3AndVarEq3() {
FieldAttribute fa = getFieldAttribute();
Expand Down Expand Up @@ -527,15 +500,4 @@ public void testEliminateRangeByEqualsInInterval() {
Expression exp = rule.rule(new And(EMPTY, eq1, r));
assertEquals(eq1, exp);
}

// 1 <= a < 10 AND a <=> 1 -> a <=> 1
public void testEliminateRangeByNullEqualsInInterval() {
FieldAttribute fa = getFieldAttribute();
NullEquals eq1 = nullEqualsOf(fa, ONE);
Range r = rangeOf(fa, ONE, true, new Literal(EMPTY, 10, DataTypes.INTEGER), false);

OptimizerRules.PropagateEquals rule = new OptimizerRules.PropagateEquals();
Expression exp = rule.rule(new And(EMPTY, eq1, r));
assertEquals(eq1, exp);
}
}