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.

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

[ES|QL] Combine Disjunctive CIDRMatch #111501

Merged
merged 11 commits into from
Aug 8, 2024
6 changes: 6 additions & 0 deletions docs/changelog/111501.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 111501
summary: "[ES|QL] Combine Disjunctive CIDRMatch"
area: ES|QL
type: enhancement
issues:
- 105143
22 changes: 22 additions & 0 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,28 @@ str1:keyword |str2:keyword |ip1:ip |ip2:ip
// end::to_ip-result[]
;

cdirMatchOrs
required_capability: combine_disjunctive_cidrmatches
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved

FROM hosts
| WHERE CIDR_MATCH(ip1, "127.0.0.2/32") or CIDR_MATCH(ip1, "127.0.0.3/32") or CIDR_MATCH(ip0, "127.0.0.1") or CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9")
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
| KEEP card, host, ip0, ip1
| sort host, card, ip0, ip1
;
warning:Line 2:20: evaluation of [ip1] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:20: java.lang.IllegalArgumentException: single-value function encountered multi-value
warning:Line 2:90: evaluation of [ip0] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:90: java.lang.IllegalArgumentException: single-value function encountered multi-value

card:keyword |host:keyword |ip0:ip |ip1:ip
eth0 |alpha |127.0.0.1 |127.0.0.1
eth0 |beta |127.0.0.1 |::1
eth1 |beta |127.0.0.1 |127.0.0.2
eth1 |beta |127.0.0.1 |128.0.0.1
eth0 |gamma |fe80::cae2:65ff:fece:feb9|127.0.0.3
lo0 |gamma |fe80::cae2:65ff:fece:feb9|fe81::cae2:65ff:fece:feb9
;

pushDownIP
from hosts | where ip1 == to_ip("::1") | keep card, host, ip0, ip1;
ignoreOrder:true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,12 @@ public enum Cap {
/**
* Add CombineBinaryComparisons rule.
*/
COMBINE_BINARY_COMPARISONS;
COMBINE_BINARY_COMPARISONS,

/**
* Support CIDRMatch in CombineDisjunctives rule.
*/
COMBINE_DISJUNCTIVE_CIDRMATCHES;

private final boolean snapshotOnly;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
import org.elasticsearch.xpack.esql.expression.function.scalar.ip.CIDRMatch;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.In;

Expand Down Expand Up @@ -48,15 +49,21 @@ protected Equals createEquals(Expression k, Set<Expression> v, ZoneId finalZoneI
return new Equals(k.source(), k, v.iterator().next(), finalZoneId);
}
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved

protected CIDRMatch createCIDRMatch(Expression k, List<Expression> v) {
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
return new CIDRMatch(k.source(), k, v);
}

@Override
public Expression rule(Or or) {
Expression e = or;
// look only at equals and In
List<Expression> exps = splitOr(e);

Map<Expression, Set<Expression>> found = new LinkedHashMap<>();
Map<Expression, Set<Expression>> cIDRMatch = new LinkedHashMap<>();
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
ZoneId zoneId = null;
List<Expression> ors = new LinkedList<>();
boolean changed = false;

for (Expression exp : exps) {
if (exp instanceof Equals eq) {
Expand All @@ -71,6 +78,8 @@ public Expression rule(Or or) {
}
} else if (exp instanceof In in) {
found.computeIfAbsent(in.value(), k -> new LinkedHashSet<>()).addAll(in.list());
} else if (exp instanceof CIDRMatch cm) {
cIDRMatch.computeIfAbsent(cm.ipField(), k -> new LinkedHashSet<>()).addAll(cm.matches());
} else {
ors.add(exp);
}
Expand All @@ -83,6 +92,15 @@ public Expression rule(Or or) {
(k, v) -> { ors.add(v.size() == 1 ? createEquals(k, v, finalZoneId) : createIn(k, new ArrayList<>(v), finalZoneId)); }
);

changed = true;
}

if (cIDRMatch.isEmpty() == false) {
cIDRMatch.forEach((k, v) -> { ors.add(createCIDRMatch(k, new ArrayList<>(v))); });
changed = true;
}

if (changed) {
// TODO: this makes a QL `or`, not an ESQL `or`
Expression combineOr = combineOr(ors);
// check the result semantically since the result might different in order
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.Literal;
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.And;
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.core.util.CollectionUtils;
import org.elasticsearch.xpack.esql.expression.function.scalar.ip.CIDRMatch;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.In;
import org.elasticsearch.xpack.esql.plan.logical.Filter;
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;

import java.util.HashSet;
import java.util.List;

import static java.util.Arrays.asList;
Expand All @@ -28,6 +33,7 @@
import static org.elasticsearch.xpack.esql.EsqlTestUtils.lessThanOf;
import static org.elasticsearch.xpack.esql.EsqlTestUtils.relation;
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
import static org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase.randomLiteral;
import static org.hamcrest.Matchers.contains;

public class CombineDisjunctionsToInTests extends ESTestCase {
Expand Down Expand Up @@ -129,4 +135,32 @@ public void testOrWithNonCombinableExpressions() {
assertEquals(fa, in.value());
assertThat(in.list(), contains(ONE, THREE));
}

public void testCombineCIDRMatch() {
FieldAttribute faa = getFieldAttribute("a");
FieldAttribute fab = getFieldAttribute("b");

List<Expression> ipa1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP));
List<Expression> ipa2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP));
List<Expression> ipb1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP));
List<Expression> ipb2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP));

List<Expression> ipa = new HashSet<>(CollectionUtils.combine(ipa1, ipa2)).stream().toList();
List<Expression> ipb = new HashSet<>(CollectionUtils.combine(ipb1, ipb2)).stream().toList();

Or or1 = new Or(EMPTY, new CIDRMatch(EMPTY, faa, ipa1), new CIDRMatch(EMPTY, faa, ipa2));
Or or2 = new Or(EMPTY, new CIDRMatch(EMPTY, fab, ipb1), new CIDRMatch(EMPTY, fab, ipb2));
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
Or oldOr = new Or(EMPTY, or1, or2);
Expression e = new CombineDisjunctionsToIn().rule(oldOr);
assertEquals(Or.class, e.getClass());
Or newOr = (Or) e;
assertEquals(CIDRMatch.class, newOr.left().getClass());
CIDRMatch cm1 = (CIDRMatch) newOr.left();
assertEquals(faa, cm1.ipField());
assertTrue(cm1.matches().size() == ipa.size() && cm1.matches().containsAll(ipa) && ipa.containsAll(cm1.matches()));
assertEquals(CIDRMatch.class, newOr.right().getClass());
CIDRMatch cm2 = (CIDRMatch) newOr.right();
assertEquals(fab, cm2.ipField());
assertTrue(cm2.matches().size() == ipb.size() && cm2.matches().containsAll(ipb) && ipb.containsAll(cm2.matches()));
}
}