From bf9126dfbffb14d9b9f9aac6772b276657b98a75 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 31 Jul 2024 21:02:42 -0400 Subject: [PATCH 1/7] support CIDRMatch in CombineDisjunctionsToIn --- .../src/main/resources/ip.csv-spec | 22 ++++++++++++ .../xpack/esql/action/EsqlCapabilities.java | 7 +++- .../rules/CombineDisjunctionsToIn.java | 18 ++++++++++ .../rules/CombineDisjunctionsToInTests.java | 34 +++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 697b1c899d65e..024a9697d302e 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -282,6 +282,28 @@ str1:keyword |str2:keyword |ip1:ip |ip2:ip // end::to_ip-result[] ; +cdirMatchOrs +required_capability: combine_disjunctive_cidrmatches + +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") +| 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 diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 6e57794bbd6aa..52e28acac0348 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -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; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java index 42d4bf730a644..02c8fc5a48647 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java @@ -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; @@ -48,6 +49,10 @@ protected Equals createEquals(Expression k, Set v, ZoneId finalZoneI return new Equals(k.source(), k, v.iterator().next(), finalZoneId); } + protected CIDRMatch createCIDRMatch(Expression k, List v) { + return new CIDRMatch(k.source(), k, v); + } + @Override public Expression rule(Or or) { Expression e = or; @@ -55,8 +60,10 @@ public Expression rule(Or or) { List exps = splitOr(e); Map> found = new LinkedHashMap<>(); + Map> cIDRMatch = new LinkedHashMap<>(); ZoneId zoneId = null; List ors = new LinkedList<>(); + boolean changed = false; for (Expression exp : exps) { if (exp instanceof Equals eq) { @@ -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); } @@ -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 diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java index 7bc2d69cb56e6..c1f330dd03460 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java @@ -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; @@ -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 { @@ -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 ipa1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipa2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + + List ipa = new HashSet<>(CollectionUtils.combine(ipa1, ipa2)).stream().toList(); + List 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)); + 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())); + } } From 20f0f4f148cd7ecea5d477c729a9a1e3bd1a9683 Mon Sep 17 00:00:00 2001 From: Fang Xing <155562079+fang-xing-esql@users.noreply.github.com> Date: Wed, 31 Jul 2024 21:08:29 -0400 Subject: [PATCH 2/7] Update docs/changelog/111501.yaml --- docs/changelog/111501.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/111501.yaml diff --git a/docs/changelog/111501.yaml b/docs/changelog/111501.yaml new file mode 100644 index 0000000000000..a424142376e52 --- /dev/null +++ b/docs/changelog/111501.yaml @@ -0,0 +1,6 @@ +pr: 111501 +summary: "[ES|QL] Combine Disjunctive CIDRMatch" +area: ES|QL +type: enhancement +issues: + - 105143 From d9a64902a1b3576279192e100030dd78fa8a1c3f Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Tue, 6 Aug 2024 13:06:23 -0400 Subject: [PATCH 3/7] update according to review comments --- .../core/planner/ExpressionTranslators.java | 8 +- .../src/main/resources/ip.csv-spec | 78 ++++- .../esql/optimizer/LogicalPlanOptimizer.java | 4 +- ...ionsToIn.java => CombineDisjunctions.java} | 66 ++-- .../rules/CombineDisjunctionsTests.java | 281 ++++++++++++++++++ .../rules/CombineDisjunctionsToInTests.java | 166 ----------- .../esql/planner/QueryTranslatorTests.java | 34 +++ 7 files changed, 445 insertions(+), 192 deletions(-) rename x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/{CombineDisjunctionsToIn.java => CombineDisjunctions.java} (52%) create mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java delete mode 100644 x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java index 2df3a8eba46d5..de7ea68f6201f 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java @@ -260,8 +260,12 @@ private static Query boolQuery(Source source, Query left, Query right, boolean i } List queries; // check if either side is already a bool query to an extra bool query - if (left instanceof BoolQuery bool && bool.isAnd() == isAnd) { - queries = CollectionUtils.combine(bool.queries(), right); + if (left instanceof BoolQuery leftBool && leftBool.isAnd() == isAnd) { + if (right instanceof BoolQuery rightBool && rightBool.isAnd() == isAnd) { + queries = CollectionUtils.combine(leftBool.queries(), rightBool.queries()); + } else { + queries = CollectionUtils.combine(leftBool.queries(), right); + } } else if (right instanceof BoolQuery bool && bool.isAnd() == isAnd) { queries = CollectionUtils.combine(bool.queries(), left); } else { diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index 024a9697d302e..abce46bd4b31f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -282,18 +282,18 @@ str1:keyword |str2:keyword |ip1:ip |ip2:ip // end::to_ip-result[] ; -cdirMatchOrs +cdirMatchOrsIPs required_capability: combine_disjunctive_cidrmatches -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") +FROM hosts +| WHERE CIDR_MATCH(ip1, "127.0.0.2/32") or CIDR_MATCH(ip0, "127.0.0.1") or CIDR_MATCH(ip1, "127.0.0.3/32") or CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") | 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 +warning:Line 2:55: evaluation of [ip0] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:55: 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 @@ -304,6 +304,74 @@ eth0 |gamma |fe80::cae2:65ff:fece:feb9|127.0.0.3 lo0 |gamma |fe80::cae2:65ff:fece:feb9|fe81::cae2:65ff:fece:feb9 ; +cdirMatchEqualsInsOrs +required_capability: combine_disjunctive_cidrmatches + +FROM hosts +| WHERE host == "alpha" OR host == "gamma" OR CIDR_MATCH(ip1, "127.0.0.2/32") OR CIDR_MATCH(ip0, "127.0.0.1") OR card IN ("eth0", "eth1") OR CIDR_MATCH(ip1, "127.0.0.3/32") OR card == "lo0" OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta" +| KEEP card, host, ip0, ip1 +| sort host, card, ip0, ip1 +; +warning:Line 2:58: evaluation of [ip1] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:58: java.lang.IllegalArgumentException: single-value function encountered multi-value +warning:Line 2:93: evaluation of [ip0] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:93: 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 +eth1 |alpha |::1 |::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 |epsilon |[fe80::cae2:65ff:fece:feb9, fe80::cae2:65ff:fece:fec0, fe80::cae2:65ff:fece:fec1] |fe80::cae2:65ff:fece:fec1 +eth1 |epsilon |null |[127.0.0.1, 127.0.0.2, 127.0.0.3] +eth0 |gamma |fe80::cae2:65ff:fece:feb9 |127.0.0.3 +lo0 |gamma |fe80::cae2:65ff:fece:feb9 |fe81::cae2:65ff:fece:feb9 +; + +cdirMatchEqualsInsOrsIPs +required_capability: combine_disjunctive_cidrmatches + +FROM hosts +| WHERE host == "alpha" OR host == "gamma" OR CIDR_MATCH(ip1, "127.0.0.2/32") OR ip0 == "127.0.0.1" OR card IN ("eth0", "eth1") OR ip1 IN ("127.0.0.3", "127.0.0.1") OR card == "lo0" OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta" +| KEEP card, host, ip0, ip1 +| sort host, card, ip0, ip1 +; +warning:Line 2:58: evaluation of [ip1] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:58: java.lang.IllegalArgumentException: single-value function encountered multi-value +warning:Line 2:82: evaluation of [ip0] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:82: 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 +eth1 |alpha |::1 |::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 |epsilon |[fe80::cae2:65ff:fece:feb9, fe80::cae2:65ff:fece:fec0, fe80::cae2:65ff:fece:fec1] |fe80::cae2:65ff:fece:fec1 +eth1 |epsilon |null |[127.0.0.1, 127.0.0.2, 127.0.0.3] +eth0 |gamma |fe80::cae2:65ff:fece:feb9 |127.0.0.3 +lo0 |gamma |fe80::cae2:65ff:fece:feb9 |fe81::cae2:65ff:fece:feb9 +; + +cdirMatchOrsWithoutCombination +required_capability: combine_disjunctive_cidrmatches + +FROM hosts +| WHERE CIDR_MATCH(ip0, "127.0.0.2") OR CIDR_MATCH(ip0, "127.0.0.3") AND CIDR_MATCH(ip1, "fe80::cae2:65ff:fece:fec0") +| STATS c = COUNT(*) +; +warning:Line 2:41: evaluation of [CIDR_MATCH(ip0, \"127.0.0.3\")] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:41: java.lang.IllegalArgumentException: single-value function encountered multi-value +warning:Line 2:74: evaluation of [CIDR_MATCH(ip1, \"fe80::cae2:65ff:fece:fec0\")] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:74: java.lang.IllegalArgumentException: single-value function encountered multi-value +warning:Line 2:9: evaluation of [CIDR_MATCH(ip0, \"127.0.0.2\")] failed, treating result as null. Only first 20 failures recorded. +warning:Line 2:9: java.lang.IllegalArgumentException: single-value function encountered multi-value + +c:long +0 +; + pushDownIP from hosts | where ip1 == to_ip("::1") | keep card, host, ip0, ip1; ignoreOrder:true diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java index bad8080c3def4..fd15aca8cf2de 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java @@ -28,7 +28,7 @@ import org.elasticsearch.xpack.esql.optimizer.rules.BooleanFunctionEqualsElimination; import org.elasticsearch.xpack.esql.optimizer.rules.BooleanSimplification; import org.elasticsearch.xpack.esql.optimizer.rules.CombineBinaryComparisons; -import org.elasticsearch.xpack.esql.optimizer.rules.CombineDisjunctionsToIn; +import org.elasticsearch.xpack.esql.optimizer.rules.CombineDisjunctions; import org.elasticsearch.xpack.esql.optimizer.rules.CombineEvals; import org.elasticsearch.xpack.esql.optimizer.rules.CombineProjections; import org.elasticsearch.xpack.esql.optimizer.rules.ConstantFolding; @@ -209,7 +209,7 @@ protected static Batch operators() { new PropagateNullable(), new BooleanFunctionEqualsElimination(), new CombineBinaryComparisons(), - new CombineDisjunctionsToIn(), + new CombineDisjunctions(), new SimplifyComparisonsArithmetics(DataType::areCompatible), // prune/elimination new PruneFilters(), diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java similarity index 52% rename from x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java rename to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java index 02c8fc5a48647..0bd5fee947929 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToIn.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java @@ -7,8 +7,12 @@ package org.elasticsearch.xpack.esql.optimizer.rules; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.Literal; import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; 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; @@ -24,52 +28,62 @@ import static org.elasticsearch.xpack.esql.core.expression.predicate.Predicates.combineOr; import static org.elasticsearch.xpack.esql.core.expression.predicate.Predicates.splitOr; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.ipToString; /** - * Combine disjunctions on the same field into an In expression. + * Combine disjunctive Equals or In expression on the same field into an In expression. * This rule looks for both simple equalities: * 1. a == 1 OR a == 2 becomes a IN (1, 2) * and combinations of In * 2. a == 1 OR a IN (2) becomes a IN (1, 2) * 3. a IN (1) OR a IN (2) becomes a IN (1, 2) + * Combine disjunctive Equals, In or CIDRMatch expressions on the same field into a CIDRMatch expression. + * 4. CIDRMatch(a, ip1) OR CIDRMatch(a, ip2) OR a = ip3 or a IN (ip4, ip5) becomes CIDRMatch(a, ip1, ip2, ip3, ip4, ip5) *

* This rule does NOT check for type compatibility as that phase has been * already be verified in the analyzer. */ -public final class CombineDisjunctionsToIn extends OptimizerRules.OptimizerExpressionRule { - public CombineDisjunctionsToIn() { +public final class CombineDisjunctions extends OptimizerRules.OptimizerExpressionRule { + public CombineDisjunctions() { super(OptimizerRules.TransformDirection.UP); } - protected In createIn(Expression key, List values, ZoneId zoneId) { + protected static In createIn(Expression key, List values, ZoneId zoneId) { return new In(key.source(), key, values); } - protected Equals createEquals(Expression k, Set v, ZoneId finalZoneId) { + protected static Equals createEquals(Expression k, Set v, ZoneId finalZoneId) { return new Equals(k.source(), k, v.iterator().next(), finalZoneId); } - protected CIDRMatch createCIDRMatch(Expression k, List v) { + protected static CIDRMatch createCIDRMatch(Expression k, List v) { return new CIDRMatch(k.source(), k, v); } @Override public Expression rule(Or or) { Expression e = or; - // look only at equals and In + // look only at equals, In and CIDRMatch List exps = splitOr(e); - Map> found = new LinkedHashMap<>(); - Map> cIDRMatch = new LinkedHashMap<>(); + Map> ins = new LinkedHashMap<>(); + Map> cidrs = new LinkedHashMap<>(); + Map> ips = new LinkedHashMap<>(); ZoneId zoneId = null; List ors = new LinkedList<>(); boolean changed = false; - for (Expression exp : exps) { if (exp instanceof Equals eq) { // consider only equals against foldables if (eq.right().foldable()) { - found.computeIfAbsent(eq.left(), k -> new LinkedHashSet<>()).add(eq.right()); + ins.computeIfAbsent(eq.left(), k -> new LinkedHashSet<>()).add(eq.right()); + if (eq.left().dataType() == DataType.IP) { + Object value = eq.right().fold(); + if (value instanceof BytesRef bytesRef) { + value = ipToString(bytesRef); + } + ips.computeIfAbsent(eq.left(), k -> new LinkedHashSet<>()).add(new Literal(Source.EMPTY, value, DataType.IP)); + } } else { ors.add(exp); } @@ -77,26 +91,44 @@ public Expression rule(Or or) { zoneId = eq.zoneId(); } } else if (exp instanceof In in) { - found.computeIfAbsent(in.value(), k -> new LinkedHashSet<>()).addAll(in.list()); + ins.computeIfAbsent(in.value(), k -> new LinkedHashSet<>()).addAll(in.list()); + if (in.value().dataType() == DataType.IP) { + List values = new ArrayList<>(in.list().size()); + for (Expression i : in.list()) { + Object value = i.fold(); + if (value instanceof BytesRef bytesRef) { + value = ipToString(bytesRef); + } + values.add(new Literal(Source.EMPTY, value, DataType.IP)); + } + ips.computeIfAbsent(in.value(), k -> new LinkedHashSet<>()).addAll(values); + } } else if (exp instanceof CIDRMatch cm) { - cIDRMatch.computeIfAbsent(cm.ipField(), k -> new LinkedHashSet<>()).addAll(cm.matches()); + cidrs.computeIfAbsent(cm.ipField(), k -> new LinkedHashSet<>()).addAll(cm.matches()); } else { ors.add(exp); } } - if (found.isEmpty() == false) { + if (cidrs.isEmpty() == false) { + for (Expression f : ips.keySet()) { + cidrs.computeIfAbsent(f, k -> new LinkedHashSet<>()).addAll(ips.get(f)); + ins.remove(f); + } + } + + if (ins.isEmpty() == false) { // combine equals alongside the existing ors final ZoneId finalZoneId = zoneId; - found.forEach( + ins.forEach( (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))); }); + if (cidrs.isEmpty() == false) { + cidrs.forEach((k, v) -> { ors.add(createCIDRMatch(k, new ArrayList<>(v))); }); changed = true; } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java new file mode 100644 index 0000000000000..dd05c1dedacd7 --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java @@ -0,0 +1,281 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.optimizer.rules; + +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.Predicates; +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.ArrayList; +import java.util.HashSet; +import java.util.List; + +import static java.util.Arrays.asList; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.FIVE; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.FOUR; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.ONE; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.SIX; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.THREE; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalsOf; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; +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 CombineDisjunctionsTests extends ESTestCase { + + public void testTwoEqualsWithOr() { + FieldAttribute fa = getFieldAttribute(); + + Or or = new Or(EMPTY, equalsOf(fa, ONE), equalsOf(fa, TWO)); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(In.class, e.getClass()); + In in = (In) e; + assertEquals(fa, in.value()); + assertThat(in.list(), contains(ONE, TWO)); + } + + public void testTwoEqualsWithSameValue() { + FieldAttribute fa = getFieldAttribute(); + + Or or = new Or(EMPTY, equalsOf(fa, ONE), equalsOf(fa, ONE)); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(Equals.class, e.getClass()); + Equals eq = (Equals) e; + assertEquals(fa, eq.left()); + assertEquals(ONE, eq.right()); + } + + public void testOneEqualsOneIn() { + FieldAttribute fa = getFieldAttribute(); + + Or or = new Or(EMPTY, equalsOf(fa, ONE), new In(EMPTY, fa, List.of(TWO))); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(In.class, e.getClass()); + In in = (In) e; + assertEquals(fa, in.value()); + assertThat(in.list(), contains(ONE, TWO)); + } + + public void testOneEqualsOneInWithSameValue() { + FieldAttribute fa = getFieldAttribute(); + + Or or = new Or(EMPTY, equalsOf(fa, ONE), new In(EMPTY, fa, asList(ONE, TWO))); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(In.class, e.getClass()); + In in = (In) e; + assertEquals(fa, in.value()); + assertThat(in.list(), contains(ONE, TWO)); + } + + public void testSingleValueInToEquals() { + FieldAttribute fa = getFieldAttribute(); + + Equals equals = equalsOf(fa, ONE); + Or or = new Or(EMPTY, equals, new In(EMPTY, fa, List.of(ONE))); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(equals, e); + } + + public void testEqualsBehindAnd() { + FieldAttribute fa = getFieldAttribute(); + + And and = new And(EMPTY, equalsOf(fa, ONE), equalsOf(fa, TWO)); + Filter dummy = new Filter(EMPTY, relation(), and); + LogicalPlan transformed = new CombineDisjunctions().apply(dummy); + assertSame(dummy, transformed); + assertEquals(and, ((Filter) transformed).condition()); + } + + public void testTwoEqualsDifferentFields() { + FieldAttribute fieldOne = getFieldAttribute("ONE"); + FieldAttribute fieldTwo = getFieldAttribute("TWO"); + + Or or = new Or(EMPTY, equalsOf(fieldOne, ONE), equalsOf(fieldTwo, TWO)); + Expression e = new CombineDisjunctions().rule(or); + assertEquals(or, e); + } + + public void testMultipleIn() { + FieldAttribute fa = getFieldAttribute(); + + Or firstOr = new Or(EMPTY, new In(EMPTY, fa, List.of(ONE)), new In(EMPTY, fa, List.of(TWO))); + Or secondOr = new Or(EMPTY, firstOr, new In(EMPTY, fa, List.of(THREE))); + Expression e = new CombineDisjunctions().rule(secondOr); + assertEquals(In.class, e.getClass()); + In in = (In) e; + assertEquals(fa, in.value()); + assertThat(in.list(), contains(ONE, TWO, THREE)); + } + + public void testOrWithNonCombinableExpressions() { + FieldAttribute fa = getFieldAttribute(); + + Or firstOr = new Or(EMPTY, new In(EMPTY, fa, List.of(ONE)), lessThanOf(fa, TWO)); + Or secondOr = new Or(EMPTY, firstOr, new In(EMPTY, fa, List.of(THREE))); + Expression e = new CombineDisjunctions().rule(secondOr); + assertEquals(Or.class, e.getClass()); + Or or = (Or) e; + assertEquals(or.left(), firstOr.right()); + assertEquals(In.class, or.right().getClass()); + In in = (In) or.right(); + assertEquals(fa, in.value()); + assertThat(in.list(), contains(ONE, THREE)); + } + + public void testCombineCIDRMatch() { + FieldAttribute faa = getFieldAttribute("a"); + FieldAttribute fab = getFieldAttribute("b"); + + List ipa1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipa2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + + List ipa = new HashSet<>(CollectionUtils.combine(ipa1, ipa2)).stream().toList(); + List ipb = new HashSet<>(CollectionUtils.combine(ipb1, ipb2)).stream().toList(); + + List cidrs = new ArrayList<>(4); + cidrs.add(new CIDRMatch(EMPTY, faa, ipa1)); + cidrs.add(new CIDRMatch(EMPTY, fab, ipb1)); + cidrs.add(new CIDRMatch(EMPTY, faa, ipa2)); + cidrs.add(new CIDRMatch(EMPTY, fab, ipb2)); + Or oldOr = (Or) Predicates.combineOr(cidrs); + Expression e = new CombineDisjunctions().rule(oldOr); + assertEquals(Or.class, e.getClass()); + Or newOr = (Or) e; + assertEquals(CIDRMatch.class, newOr.left().getClass()); + assertEquals(CIDRMatch.class, newOr.right().getClass()); + + CIDRMatch cm1 = (CIDRMatch) newOr.left(); + CIDRMatch cm2 = (CIDRMatch) newOr.right(); + + if (cm1.ipField() == faa) { + assertEquals(fab, cm2.ipField()); + assertTrue(cm1.matches().size() == ipa.size() && cm1.matches().containsAll(ipa) && ipa.containsAll(cm1.matches())); + assertTrue(cm2.matches().size() == ipb.size() && cm2.matches().containsAll(ipb) && ipb.containsAll(cm2.matches())); + } else { + assertEquals(fab, cm1.ipField()); + assertTrue(cm1.matches().size() == ipb.size() && cm1.matches().containsAll(ipb) && ipb.containsAll(cm1.matches())); + assertTrue(cm2.matches().size() == ipa.size() && cm2.matches().containsAll(ipa) && ipa.containsAll(cm2.matches())); + } + } + + public void testCombineCIDRMatchEqualsIns() { + FieldAttribute fa_cidr_a = getFieldAttribute("cidr_a"); + FieldAttribute fa_cidr_b = getFieldAttribute("cidr_b"); + FieldAttribute fa_in_a = getFieldAttribute("in_a"); + FieldAttribute fa_eqin_a = getFieldAttribute("eqin_a"); + FieldAttribute fa_eq_a = getFieldAttribute("eq_a"); + + List ipa1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipa2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + List ipb2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); + + List ipa = new HashSet<>(CollectionUtils.combine(ipa1, ipa2)).stream().toList(); + List ipb = new HashSet<>(CollectionUtils.combine(ipb1, ipb2)).stream().toList(); + + List all = new ArrayList<>(10); + all.add(new CIDRMatch(EMPTY, fa_cidr_a, ipa1)); + all.add(new CIDRMatch(EMPTY, fa_cidr_b, ipb1)); + all.add(new CIDRMatch(EMPTY, fa_cidr_a, ipa2)); + all.add(new CIDRMatch(EMPTY, fa_cidr_b, ipb2)); + + all.add(new In(EMPTY, fa_in_a, List.of(ONE, TWO, THREE))); + all.add(new In(EMPTY, fa_eqin_a, List.of(FOUR, FIVE, SIX))); + all.add(new In(EMPTY, fa_in_a, List.of(THREE, FOUR, FIVE))); + + all.add(new Equals(EMPTY, fa_eq_a, ONE)); + all.add(new Equals(EMPTY, fa_eqin_a, ONE)); + all.add(new Equals(EMPTY, fa_eq_a, SIX)); + + Or oldOr = (Or) Predicates.combineOr(all); + + Expression e = new CombineDisjunctions().rule(oldOr); + assertEquals(Or.class, e.getClass()); + Or newOr = (Or) e; + assertEquals(Or.class, newOr.left().getClass()); + Or newOr1 = (Or) newOr.left(); + assertEquals(CIDRMatch.class, newOr.right().getClass()); + CIDRMatch cidr2 = (CIDRMatch) newOr.right(); + assertEquals(Or.class, e.getClass()); + + assertEquals(Or.class, newOr1.left().getClass()); + Or newOr2 = (Or) newOr1.left(); + assertEquals(In.class, newOr2.left().getClass()); + In in1 = (In) newOr2.left(); + assertEquals(In.class, newOr2.right().getClass()); + In in2 = (In) newOr2.right(); + assertEquals(Or.class, newOr1.right().getClass()); + Or newOr3 = (Or) newOr1.right(); + assertEquals(In.class, newOr3.left().getClass()); + In in3 = (In) newOr3.left(); + assertEquals(CIDRMatch.class, newOr3.right().getClass()); + CIDRMatch cidr1 = (CIDRMatch) newOr3.right(); + + if (cidr1.ipField() == fa_cidr_a) { + assertEquals(fa_cidr_b, cidr2.ipField()); + assertTrue(cidr1.matches().size() == ipa.size() && cidr1.matches().containsAll(ipa) && ipa.containsAll(cidr1.matches())); + assertTrue(cidr2.matches().size() == ipb.size() && cidr2.matches().containsAll(ipb) && ipb.containsAll(cidr2.matches())); + } else { + assertEquals(fa_cidr_b, cidr1.ipField()); + assertTrue(cidr1.matches().size() == ipb.size() && cidr1.matches().containsAll(ipb) && ipb.containsAll(cidr1.matches())); + assertTrue(cidr2.matches().size() == ipa.size() && cidr2.matches().containsAll(ipa) && ipa.containsAll(cidr2.matches())); + } + + if (in1.value() == fa_in_a) { + assertTrue(in1.list().size() == 5 && in1.list().containsAll(List.of(ONE, TWO, THREE, FOUR, FIVE))); + if (in2.value() == fa_eqin_a) { + assertEquals(in3.value(), fa_eq_a); + assertTrue(in3.list().size() == 2 && in3.list().containsAll(List.of(ONE, SIX))); + assertTrue(in2.list().size() == 4 && in2.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } else { + assertEquals(in2.value(), fa_eq_a); + assertTrue(in2.list().size() == 2 && in2.list().containsAll(List.of(ONE, SIX))); + assertTrue(in3.list().size() == 4 && in3.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } + } else if (in2.value() == fa_in_a) { + assertTrue(in2.list().size() == 5 && in2.list().containsAll(List.of(ONE, TWO, THREE, FOUR, FIVE))); + if (in1.value() == fa_eqin_a) { + assertEquals(in3.value(), fa_eq_a); + assertTrue(in3.list().size() == 2 && in3.list().containsAll(List.of(ONE, SIX))); + assertTrue(in1.list().size() == 4 && in1.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } else { + assertEquals(in3.value(), fa_eq_a); + assertTrue(in3.list().size() == 2 && in3.list().containsAll(List.of(ONE, SIX))); + assertTrue(in1.list().size() == 4 && in1.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } + } else { + assertTrue(in3.list().size() == 5 && in3.list().containsAll(List.of(ONE, TWO, THREE, FOUR, FIVE))); + if (in1.value() == fa_eqin_a) { + assertEquals(in2.value(), fa_eq_a); + assertTrue(in2.list().size() == 2 && in2.list().containsAll(List.of(ONE, SIX))); + assertTrue(in1.list().size() == 4 && in1.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } else { + assertEquals(in1.value(), fa_eq_a); + assertTrue(in1.list().size() == 2 && in1.list().containsAll(List.of(ONE, SIX))); + assertTrue(in2.list().size() == 4 && in2.list().containsAll(List.of(ONE, FOUR, FIVE, SIX))); + } + } + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java deleted file mode 100644 index c1f330dd03460..0000000000000 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsToInTests.java +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.esql.optimizer.rules; - -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; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.ONE; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.THREE; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.TWO; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalsOf; -import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; -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 { - public void testTwoEqualsWithOr() { - FieldAttribute fa = getFieldAttribute(); - - Or or = new Or(EMPTY, equalsOf(fa, ONE), equalsOf(fa, TWO)); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(In.class, e.getClass()); - In in = (In) e; - assertEquals(fa, in.value()); - assertThat(in.list(), contains(ONE, TWO)); - } - - public void testTwoEqualsWithSameValue() { - FieldAttribute fa = getFieldAttribute(); - - Or or = new Or(EMPTY, equalsOf(fa, ONE), equalsOf(fa, ONE)); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(Equals.class, e.getClass()); - Equals eq = (Equals) e; - assertEquals(fa, eq.left()); - assertEquals(ONE, eq.right()); - } - - public void testOneEqualsOneIn() { - FieldAttribute fa = getFieldAttribute(); - - Or or = new Or(EMPTY, equalsOf(fa, ONE), new In(EMPTY, fa, List.of(TWO))); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(In.class, e.getClass()); - In in = (In) e; - assertEquals(fa, in.value()); - assertThat(in.list(), contains(ONE, TWO)); - } - - public void testOneEqualsOneInWithSameValue() { - FieldAttribute fa = getFieldAttribute(); - - Or or = new Or(EMPTY, equalsOf(fa, ONE), new In(EMPTY, fa, asList(ONE, TWO))); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(In.class, e.getClass()); - In in = (In) e; - assertEquals(fa, in.value()); - assertThat(in.list(), contains(ONE, TWO)); - } - - public void testSingleValueInToEquals() { - FieldAttribute fa = getFieldAttribute(); - - Equals equals = equalsOf(fa, ONE); - Or or = new Or(EMPTY, equals, new In(EMPTY, fa, List.of(ONE))); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(equals, e); - } - - public void testEqualsBehindAnd() { - FieldAttribute fa = getFieldAttribute(); - - And and = new And(EMPTY, equalsOf(fa, ONE), equalsOf(fa, TWO)); - Filter dummy = new Filter(EMPTY, relation(), and); - LogicalPlan transformed = new CombineDisjunctionsToIn().apply(dummy); - assertSame(dummy, transformed); - assertEquals(and, ((Filter) transformed).condition()); - } - - public void testTwoEqualsDifferentFields() { - FieldAttribute fieldOne = getFieldAttribute("ONE"); - FieldAttribute fieldTwo = getFieldAttribute("TWO"); - - Or or = new Or(EMPTY, equalsOf(fieldOne, ONE), equalsOf(fieldTwo, TWO)); - Expression e = new CombineDisjunctionsToIn().rule(or); - assertEquals(or, e); - } - - public void testMultipleIn() { - FieldAttribute fa = getFieldAttribute(); - - Or firstOr = new Or(EMPTY, new In(EMPTY, fa, List.of(ONE)), new In(EMPTY, fa, List.of(TWO))); - Or secondOr = new Or(EMPTY, firstOr, new In(EMPTY, fa, List.of(THREE))); - Expression e = new CombineDisjunctionsToIn().rule(secondOr); - assertEquals(In.class, e.getClass()); - In in = (In) e; - assertEquals(fa, in.value()); - assertThat(in.list(), contains(ONE, TWO, THREE)); - } - - public void testOrWithNonCombinableExpressions() { - FieldAttribute fa = getFieldAttribute(); - - Or firstOr = new Or(EMPTY, new In(EMPTY, fa, List.of(ONE)), lessThanOf(fa, TWO)); - Or secondOr = new Or(EMPTY, firstOr, new In(EMPTY, fa, List.of(THREE))); - Expression e = new CombineDisjunctionsToIn().rule(secondOr); - assertEquals(Or.class, e.getClass()); - Or or = (Or) e; - assertEquals(or.left(), firstOr.right()); - assertEquals(In.class, or.right().getClass()); - In in = (In) or.right(); - assertEquals(fa, in.value()); - assertThat(in.list(), contains(ONE, THREE)); - } - - public void testCombineCIDRMatch() { - FieldAttribute faa = getFieldAttribute("a"); - FieldAttribute fab = getFieldAttribute("b"); - - List ipa1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); - List ipa2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); - List ipb1 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); - List ipb2 = randomList(1, 5, () -> new Literal(EMPTY, randomLiteral(DataType.IP).value(), DataType.IP)); - - List ipa = new HashSet<>(CollectionUtils.combine(ipa1, ipa2)).stream().toList(); - List 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)); - 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())); - } -} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java index 5954836096ffa..5a42012f16c91 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java @@ -31,10 +31,13 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.matchesRegex; +//@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE", reason = "debug") public class QueryTranslatorTests extends ESTestCase { private static TestPlannerOptimizer plannerOptimizer; + private static TestPlannerOptimizer plannerOptimizerIPs; + private static Analyzer makeAnalyzer(String mappingFileName) { var mapping = loadMapping(mappingFileName); EsIndex test = new EsIndex("test", mapping, Set.of("test")); @@ -49,6 +52,7 @@ private static Analyzer makeAnalyzer(String mappingFileName) { @BeforeClass public static void init() { plannerOptimizer = new TestPlannerOptimizer(EsqlTestUtils.TEST_CFG, makeAnalyzer("mapping-all-types.json")); + plannerOptimizerIPs = new TestPlannerOptimizer(EsqlTestUtils.TEST_CFG, makeAnalyzer("mapping-hosts.json")); } @Override @@ -63,6 +67,13 @@ public void assertQueryTranslation(String query, Matcher translationMatc assertThat(translatedQuery, translationMatcher); } + public void assertQueryTranslationIPs(String query, Matcher translationMatcher) { + PhysicalPlan optimized = plannerOptimizerIPs.plan(query); + EsQueryExec eqe = (EsQueryExec) optimized.collectLeaves().get(0); + final String translatedQuery = eqe.query().toString().replaceAll("\\s+", ""); + assertThat(translatedQuery, translationMatcher); + } + public void testBinaryComparisons() { assertQueryTranslation(""" FROM test | WHERE 10 < integer""", containsString(""" @@ -152,4 +163,27 @@ public void testRanges() { .*must.*esql_single_value":\\{"field":"unsigned_long\"""" + """ .*"range":\\{"unsigned_long":\\{"gte":2147483648,.*"range":\\{"unsigned_long":\\{"lte":2147483650,.*""")); } + + public void testIPs() { + // Combine Equals, In and CIDRMatch on IP type + assertQueryTranslationIPs(""" + FROM hosts | WHERE host == "alpha" OR host == "gamma" OR CIDR_MATCH(ip1, "127.0.0.2/32") OR CIDR_MATCH(ip1, "127.0.0.3/32") \ + OR card IN ("eth0", "eth1") OR card == "lo0" OR CIDR_MATCH(ip0, "127.0.0.1") OR \ + CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"host".*"terms":\\{"host":\\["alpha","gamma","beta".*""" + """ + esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.2/32","127.0.0.3/32".*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); + + assertQueryTranslationIPs(""" + FROM hosts | WHERE host == "alpha" OR host == "gamma" OR ip1 IN ("127.0.0.2"::ip) OR CIDR_MATCH(ip1, "127.0.0.3/32") \ + OR card IN ("eth0", "eth1") OR card == "lo0" OR ip0 IN ("127.0.0.1"::ip, "128.0.0.1"::ip) \ + OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"host".*"terms":\\{"host":\\["alpha","gamma","beta".*""" + """ + esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.3/32","127.0.0.2".*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","128.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); + } } From a9994349e4781f6cdb4e915414d20944162a4b74 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Tue, 6 Aug 2024 14:08:52 -0400 Subject: [PATCH 4/7] fix tests --- .../src/main/resources/ip.csv-spec | 18 ------------------ .../esql/planner/QueryTranslatorTests.java | 10 ++++++++++ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec index abce46bd4b31f..0fb6994ef759f 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/ip.csv-spec @@ -354,24 +354,6 @@ eth0 |gamma |fe80::cae2:65ff:fece:feb9 lo0 |gamma |fe80::cae2:65ff:fece:feb9 |fe81::cae2:65ff:fece:feb9 ; -cdirMatchOrsWithoutCombination -required_capability: combine_disjunctive_cidrmatches - -FROM hosts -| WHERE CIDR_MATCH(ip0, "127.0.0.2") OR CIDR_MATCH(ip0, "127.0.0.3") AND CIDR_MATCH(ip1, "fe80::cae2:65ff:fece:fec0") -| STATS c = COUNT(*) -; -warning:Line 2:41: evaluation of [CIDR_MATCH(ip0, \"127.0.0.3\")] failed, treating result as null. Only first 20 failures recorded. -warning:Line 2:41: java.lang.IllegalArgumentException: single-value function encountered multi-value -warning:Line 2:74: evaluation of [CIDR_MATCH(ip1, \"fe80::cae2:65ff:fece:fec0\")] failed, treating result as null. Only first 20 failures recorded. -warning:Line 2:74: java.lang.IllegalArgumentException: single-value function encountered multi-value -warning:Line 2:9: evaluation of [CIDR_MATCH(ip0, \"127.0.0.2\")] failed, treating result as null. Only first 20 failures recorded. -warning:Line 2:9: java.lang.IllegalArgumentException: single-value function encountered multi-value - -c:long -0 -; - pushDownIP from hosts | where ip1 == to_ip("::1") | keep card, host, ip0, ip1; ignoreOrder:true diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java index 5a42012f16c91..3266a467ecdde 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java @@ -165,6 +165,16 @@ public void testRanges() { } public void testIPs() { + // Nothing to combine + assertQueryTranslationIPs(""" + FROM hosts | WHERE CIDR_MATCH(ip0, "127.0.0.1") OR CIDR_MATCH(ip0, "127.0.0.3") AND CIDR_MATCH(ip1, "fe80::cae2:65ff:fece:fec0") + """, matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1".*""" + """ + .*bool.*must.*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.3".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["fe80::cae2:65ff:fece:fec0".*""")); + // Combine Equals, In and CIDRMatch on IP type assertQueryTranslationIPs(""" FROM hosts | WHERE host == "alpha" OR host == "gamma" OR CIDR_MATCH(ip1, "127.0.0.2/32") OR CIDR_MATCH(ip1, "127.0.0.3/32") \ From 1c5eba14b2509baa9fd44723473b323a10caba4b Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 7 Aug 2024 12:19:10 -0400 Subject: [PATCH 5/7] update according to review comments --- .../esql/optimizer/rules/CombineDisjunctions.java | 11 +++++++++-- .../optimizer/rules/CombineDisjunctionsTests.java | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java index 0bd5fee947929..a691a30b3dc6c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java @@ -31,13 +31,13 @@ import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.ipToString; /** - * Combine disjunctive Equals or In expression on the same field into an In expression. + * Combine disjunctive Equals, In or CIDRMatch expressions on the same field into an In or CIDRMatch expression. * This rule looks for both simple equalities: * 1. a == 1 OR a == 2 becomes a IN (1, 2) * and combinations of In * 2. a == 1 OR a IN (2) becomes a IN (1, 2) * 3. a IN (1) OR a IN (2) becomes a IN (1, 2) - * Combine disjunctive Equals, In or CIDRMatch expressions on the same field into a CIDRMatch expression. + * and combinations of CIDRMatch * 4. CIDRMatch(a, ip1) OR CIDRMatch(a, ip2) OR a = ip3 or a IN (ip4, ip5) becomes CIDRMatch(a, ip1, ip2, ip3, ip4, ip5) *

* This rule does NOT check for type compatibility as that phase has been @@ -79,6 +79,12 @@ public Expression rule(Or or) { ins.computeIfAbsent(eq.left(), k -> new LinkedHashSet<>()).add(eq.right()); if (eq.left().dataType() == DataType.IP) { Object value = eq.right().fold(); + // ImplicitCasting and ConstantFolding(includes explicit casting) are applied before CombineDisjunctions. + // They fold the input IP string to an internal IP format. These happen to Equals and IN, but not for CIDRMatch, + // as CIDRMatch takes strings as input, ImplicitCasting does not apply to it, and the first input to CIDRMatch is a + // field, ConstantFolding does not apply to it either. + // If the data type is IP, convert the internal IP format in Equals and IN to the format that is compatible with + // CIDRMatch, and store them in a separate map, so that they can be combined into existing CIDRMatch later. if (value instanceof BytesRef bytesRef) { value = ipToString(bytesRef); } @@ -96,6 +102,7 @@ public Expression rule(Or or) { List values = new ArrayList<>(in.list().size()); for (Expression i : in.list()) { Object value = i.fold(); + // Same as Equals. if (value instanceof BytesRef bytesRef) { value = ipToString(bytesRef); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java index dd05c1dedacd7..2060327f1e18d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctionsTests.java @@ -36,9 +36,9 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalsOf; import static org.elasticsearch.xpack.esql.EsqlTestUtils.getFieldAttribute; import static org.elasticsearch.xpack.esql.EsqlTestUtils.lessThanOf; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral; 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 CombineDisjunctionsTests extends ESTestCase { From 68780f9693d54aeb384b3ff91c1b6f59318029b2 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 7 Aug 2024 23:00:50 -0400 Subject: [PATCH 6/7] merge main --- .../elasticsearch/xpack/esql/action/EsqlCapabilities.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 50b2f3771e3b7..42eebffc0375b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -213,7 +213,12 @@ public enum Cap { /** * Support for nanosecond dates as a data type */ - DATE_NANOS_TYPE(EsqlCorePlugin.DATE_NANOS_FEATURE_FLAG); + DATE_NANOS_TYPE(EsqlCorePlugin.DATE_NANOS_FEATURE_FLAG), + + /** + * Support CIDRMatch in CombineDisjunctions rule. + */ + COMBINE_DISJUNCTIVE_CIDRMATCHES; private final boolean snapshotOnly; private final FeatureFlag featureFlag; From 677118203704fc352eaa508c20cb14b344c1b0cb Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 7 Aug 2024 23:57:06 -0400 Subject: [PATCH 7/7] update according to review comments --- .../optimizer/rules/CombineDisjunctions.java | 2 +- .../LocalPhysicalPlanOptimizerTests.java | 51 ------------------- .../esql/planner/QueryTranslatorTests.java | 33 ++++++++++++ 3 files changed, 34 insertions(+), 52 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java index a691a30b3dc6c..ceac1aa9ca75b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/CombineDisjunctions.java @@ -38,7 +38,7 @@ * 2. a == 1 OR a IN (2) becomes a IN (1, 2) * 3. a IN (1) OR a IN (2) becomes a IN (1, 2) * and combinations of CIDRMatch - * 4. CIDRMatch(a, ip1) OR CIDRMatch(a, ip2) OR a = ip3 or a IN (ip4, ip5) becomes CIDRMatch(a, ip1, ip2, ip3, ip4, ip5) + * 4. CIDRMatch(a, ip1) OR CIDRMatch(a, ip2) OR a == ip3 or a IN (ip4, ip5) becomes CIDRMatch(a, ip1, ip2, ip3, ip4, ip5) *

* This rule does NOT check for type compatibility as that phase has been * already be verified in the analyzer. diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index 614f6a31a88f9..5fba11c13561c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -11,7 +11,6 @@ import org.apache.lucene.search.IndexSearcher; import org.elasticsearch.Build; -import org.elasticsearch.common.network.NetworkAddress; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.mapper.MapperService; @@ -64,15 +63,12 @@ import org.junit.Before; import java.io.IOException; -import java.util.ArrayList; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static java.util.Arrays.asList; -import static org.elasticsearch.common.logging.LoggerMessageFormat.format; import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; import static org.elasticsearch.xpack.esql.EsqlTestUtils.configuration; import static org.elasticsearch.xpack.esql.EsqlTestUtils.loadMapping; @@ -670,44 +666,6 @@ public void testIsNotNull_TextField_Pushdown_WithCount() { assertThat(stat.query(), is(QueryBuilders.existsQuery("job"))); } - /** - * Expects - * LimitExec[1000[INTEGER]] - * \_ExchangeExec[[],false] - * \_ProjectExec[[!alias_integer, boolean{f}#4, byte{f}#5, constant_keyword-foo{f}#6, date{f}#7, double{f}#8, float{f}#9, - * half_float{f}#10, integer{f}#12, ip{f}#13, keyword{f}#14, long{f}#15, scaled_float{f}#11, short{f}#17, text{f}#18, - * unsigned_long{f}#16, version{f}#19, wildcard{f}#20]] - * \_FieldExtractExec[!alias_integer, boolean{f}#4, byte{f}#5, constant_k..][] - * \_EsQueryExec[test], query[{"esql_single_value":{"field":"ip","next":{"terms":{"ip":["127.0.0.0/24"],"boost":1.0}},"source": - * "cidr_match(ip, \"127.0.0.0/24\")@1:19"}}][_doc{f}#21], limit[1000], sort[] estimatedRowSize[389] - */ - public void testCidrMatchPushdownFilter() { - var allTypeMappingAnalyzer = makeAnalyzer("mapping-ip.json", new EnrichResolution()); - final String fieldName = "ip_addr"; - - int cidrBlockCount = randomIntBetween(1, 10); - ArrayList cidrBlocks = new ArrayList<>(); - for (int i = 0; i < cidrBlockCount; i++) { - cidrBlocks.add(randomCidrBlock()); - } - String cidrBlocksString = cidrBlocks.stream().map((s) -> "\"" + s + "\"").collect(Collectors.joining(",")); - String cidrMatch = format(null, "cidr_match({}, {})", fieldName, cidrBlocksString); - - var query = "from test | where " + cidrMatch; - var plan = plannerOptimizer.plan(query, EsqlTestUtils.TEST_SEARCH_STATS, allTypeMappingAnalyzer); - - var limit = as(plan, LimitExec.class); - var exchange = as(limit.child(), ExchangeExec.class); - var project = as(exchange.child(), ProjectExec.class); - var field = as(project.child(), FieldExtractExec.class); - var queryExec = as(field.child(), EsQueryExec.class); - assertThat(queryExec.limit().fold(), is(1000)); - - var expectedInnerQuery = QueryBuilders.termsQuery(fieldName, cidrBlocks); - var expectedQuery = wrapWithSingleQuery(query, expectedInnerQuery, fieldName, new Source(1, 18, cidrMatch)); - assertThat(queryExec.query().toString(), is(expectedQuery.toString())); - } - private record OutOfRangeTestCase(String fieldName, String tooLow, String tooHigh) {}; public void testOutOfRangeFilterPushdown() { @@ -939,13 +897,4 @@ private Stat queryStatsFor(PhysicalPlan plan) { protected List filteredWarnings() { return withDefaultLimitWarning(super.filteredWarnings()); } - - private String randomCidrBlock() { - boolean ipv4 = randomBoolean(); - - String address = NetworkAddress.format(randomIp(ipv4)); - int cidrPrefixLength = ipv4 ? randomIntBetween(0, 32) : randomIntBetween(0, 128); - - return format(null, "{}/{}", address, cidrPrefixLength); - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java index bc9634bb2b170..fee260cc7c657 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/planner/QueryTranslatorTests.java @@ -186,6 +186,19 @@ OR card IN ("eth0", "eth1") OR card == "lo0" OR CIDR_MATCH(ip0, "127.0.0.1") OR esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.2/32","127.0.0.3/32".*""" + """ esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); + assertQueryTranslationIPs(""" + FROM hosts | WHERE host == "alpha" OR host == "gamma" OR CIDR_MATCH(ip1, "127.0.0.2/32") OR CIDR_MATCH(ip1, "127.0.0.3/32") \ + OR card IN ("eth0", "eth1") OR card == "lo0" OR CIDR_MATCH(ip0, "127.0.0.1") OR \ + CIDR_MATCH(ip0, "127.0.0.0/24", "172.0.0.0/31") OR CIDR_MATCH(ip0, "127.0.1.0/24") OR \ + CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:fec0", "172.0.2.0/24") OR \ + CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"host".*"terms":\\{"host":\\["alpha","gamma","beta".*""" + """ + esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.2/32","127.0.0.3/32".*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","127.0.0.0/24","172.0.0.0/31","127.0.1.0/24",\ + "fe80::cae2:65ff:fece:fec0","172.0.2.0/24","fe80::cae2:65ff:fece:feb9".*""")); + assertQueryTranslationIPs(""" FROM hosts | WHERE host == "alpha" OR host == "gamma" OR ip1 IN ("127.0.0.2"::ip) OR CIDR_MATCH(ip1, "127.0.0.3/32") \ OR card IN ("eth0", "eth1") OR card == "lo0" OR ip0 IN ("127.0.0.1"::ip, "128.0.0.1"::ip) \ @@ -195,5 +208,25 @@ OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRe esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.3/32","127.0.0.2".*""" + """ esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","128.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); + + assertQueryTranslationIPs(""" + FROM hosts | WHERE host == "alpha" OR host == "gamma" OR ip1 == "127.0.0.2"::ip OR CIDR_MATCH(ip1, "127.0.0.3/32") \ + OR card IN ("eth0", "eth1") OR card == "lo0" OR ip0 IN ("127.0.0.1"::ip, "128.0.0.1"::ip) \ + OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"host".*"terms":\\{"host":\\["alpha","gamma","beta".*""" + """ + esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.3/32","127.0.0.2".*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","128.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); + + assertQueryTranslationIPs(""" + FROM hosts | WHERE host == "alpha" OR host == "gamma" OR ip1 == "127.0.0.2" OR CIDR_MATCH(ip1, "127.0.0.3/32") \ + OR card IN ("eth0", "eth1") OR card == "lo0" OR ip0 IN ("127.0.0.1"::ip, "128.0.0.1"::ip) \ + OR CIDR_MATCH(ip0, "fe80::cae2:65ff:fece:feb9") OR host == "beta\"""", matchesRegex(""" + .*bool.*should.*""" + """ + esql_single_value":\\{"field":"host".*"terms":\\{"host":\\["alpha","gamma","beta".*""" + """ + esql_single_value":\\{"field":"card".*"terms":\\{"card":\\["eth0","eth1","lo0".*""" + """ + esql_single_value":\\{"field":"ip1".*"terms":\\{"ip1":\\["127.0.0.3/32","127.0.0.2".*""" + """ + esql_single_value":\\{"field":"ip0".*"terms":\\{"ip0":\\["127.0.0.1","128.0.0.1","fe80::cae2:65ff:fece:feb9".*""")); } }