From 218f0675d7e9359669722c607254f6c2ea01418c Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Wed, 6 Aug 2025 16:54:47 -0400 Subject: [PATCH 1/3] just knocking some stuff together --- .../xpack/esql/PartialPlanMatcher.java | 96 +++++++ .../elasticsearch/xpack/esql/PlanBuilder.java | 255 ++++++++++++++++++ .../xpack/esql/PlanContainsMatcher.java | 62 +++++ .../optimizer/LogicalPlanOptimizerTests.java | 15 +- 4 files changed, 420 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanBuilder.java create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java new file mode 100644 index 0000000000000..134e975d36cbe --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java @@ -0,0 +1,96 @@ +/* + * 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; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; + +import java.util.List; + +/** + * A matcher that compares a plan with a partial plan. + */ +public class PartialPlanMatcher extends TypeSafeMatcher { + private final LogicalPlan expected; + + public static PartialPlanMatcher matchesPartialPlan(PlanBuilder expected) { + return new PartialPlanMatcher(expected.build()); + } + + public static PartialPlanMatcher matchesPartialPlan(LogicalPlan expected) { + return new PartialPlanMatcher(expected); + } + + PartialPlanMatcher(LogicalPlan expected) { + this.expected = expected; + } + + @Override + protected boolean matchesSafely(LogicalPlan actual) { + return matches(expected, actual); + } + + @Override + public void describeTo(Description description) { + + } + + static boolean matches(LogicalPlan expected, LogicalPlan actual) { + if (expected.getClass() != actual.getClass()) { + assert false : "Mismatch in plan types: Expected [" + expected.getClass() + "], found [" + actual.getClass() + "]"; + return false; + } + + List leftProperties = expected.nodeProperties(); + List rightProperties = actual.nodeProperties(); + + for (int i = 0; i < leftProperties.size(); i++) { + Object leftProperty = leftProperties.get(i); + Object rightProperty = rightProperties.get(i); + + // Only check equality if left is not null. This way we can be lenient by simply + // leaving out properties. + if (leftProperty == null || leftProperty instanceof PlanBuilder.Ignore) { + continue; + } + + if (rightProperty == null) { + assert false : "Expected [" + leftProperty.getClass() + "], found null."; + return false; + } + + if (leftProperty instanceof LogicalPlan l) { + if (rightProperty instanceof LogicalPlan rightPlan) { + if (matches(l, rightPlan) == false) { + return false; + } + } else { + assert false + : "Mismatch in types: Expected [" + leftProperty.getClass() + "], found [" + rightProperty.getClass() + "]"; + return false; + } + } else if (leftProperty instanceof Expression e) { + if (rightProperty instanceof Expression rightExpr) { + if (e.semanticEquals(rightExpr) == false) { + assert false : "Mismatch in expressions: Expected [" + e + "], found [" + rightExpr + "]"; + return false; + } + } else { + return false; + } + } else if (leftProperty.equals(rightProperty) == false) { + assert false : "Mismatch in properties: Expected [" + leftProperty + "], found [" + rightProperty + "]"; + return false; + } + } + + return true; + } +} diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanBuilder.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanBuilder.java new file mode 100644 index 0000000000000..c222194dc1112 --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanBuilder.java @@ -0,0 +1,255 @@ +/* + * 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; + +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.xpack.esql.core.expression.Attribute; +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.core.expression.Literal; +import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.core.util.Holder; +import org.elasticsearch.xpack.esql.index.EsIndex; +import org.elasticsearch.xpack.esql.plan.logical.EsRelation; +import org.elasticsearch.xpack.esql.plan.logical.Filter; +import org.elasticsearch.xpack.esql.plan.logical.Limit; +import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.elasticsearch.xpack.esql.plan.logical.OrderBy; +import org.elasticsearch.xpack.esql.plan.logical.TopN; +import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; +import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; + +import java.lang.reflect.ParameterizedType; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.ListIterator; +import java.util.Map; + +public class PlanBuilder { + private final LogicalPlan currentPlan; + private final List currentExpressions = new ArrayList<>(); + private final List> currentExpressionLists = new ArrayList<>(); + + private PlanBuilder(LogicalPlan plan) { + this.currentPlan = plan; + } + + public LogicalPlan build() { + return currentPlan; + } + + public PlanBuilder withChild(PlanBuilder child) { + return withChild(child.currentPlan); + } + + public PlanBuilder withChild(LogicalPlan child) { + List children = currentPlan.children(); + if (children.size() == 1 && children.get(0) instanceof MockChild) { + children = new ArrayList<>(); + } + + children.add(child); + return new PlanBuilder(currentPlan.replaceChildren(children)); + } + + public PlanBuilder withExpression(Expression expression) { + currentExpressions.add(expression); + + Holder i = new Holder<>(0); + return new PlanBuilder(currentPlan.transformPropertiesOnly(Expression.class, expr -> { + int idx = i.get(); + if (idx < currentExpressions.size() == false) { + return expr; + } + i.set(idx + 1); + return currentExpressions.get(idx); + })); + } + + public PlanBuilder withExpressionList(List expressions) { + currentExpressionLists.add(expressions); + + Holder i = new Holder<>(0); + return new PlanBuilder(currentPlan.transformPropertiesOnly(List.class, prop -> { + if (prop instanceof ParameterizedType pt + && pt.getRawType() == List.class + && pt.getActualTypeArguments()[0].getClass().isAssignableFrom(Expression.class)) { + int idx = i.get(); + if (idx < currentExpressionLists.size() == false) { + return prop; + } + i.set(idx + 1); + return currentExpressionLists.get(idx); + } + return prop; + })); + } + + public static PlanBuilder limit() { + return new PlanBuilder(new Limit(null, new MockExpression(), new MockChild())); + } + + public static PlanBuilder relation() { + return new PlanBuilder(new EsRelation((Source) null, (EsIndex) null, (IndexMode) null)); + } + + public static PlanBuilder localRelation() { + return new PlanBuilder(new LocalRelation((Source) null, new MockList<>(), null)); + } + + public static PlanBuilder localRelation(List output, LocalSupplier supplier) { + return new PlanBuilder(new LocalRelation((Source) null, output, supplier)); + } + + public static PlanBuilder filter() { + return new PlanBuilder(new Filter((Source) null, new MockChild(), new MockExpression())); + } + + public static PlanBuilder orderBy() { + return new PlanBuilder(new OrderBy((Source) null, new MockChild(), new MockList<>())); + } + + public static PlanBuilder topN() { + return new PlanBuilder(new TopN((Source) null, new MockChild(), new MockList<>(), new MockExpression())); + } + + interface Ignore {} + + private static class MockChild extends EsRelation implements Ignore { + MockChild() { + super(Source.EMPTY, new EsIndex("mock_index", Map.of()), IndexMode.STANDARD); + } + } + + private static class MockExpression extends Literal implements Ignore { + MockExpression() { + super(Source.EMPTY, 0, DataType.NULL); + } + } + + private static class MockList implements List, Ignore { + MockList() {} + + @Override + public int size() { + return 0; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public boolean contains(Object o) { + return false; + } + + @Override + public Iterator iterator() { + return null; + } + + @Override + public Object[] toArray() { + return new Object[0]; + } + + @Override + public T[] toArray(T[] a) { + return null; + } + + @Override + public boolean add(E e) { + return false; + } + + @Override + public boolean remove(Object o) { + return false; + } + + @Override + public boolean containsAll(Collection c) { + return false; + } + + @Override + public boolean addAll(Collection c) { + return false; + } + + @Override + public boolean addAll(int index, Collection c) { + return false; + } + + @Override + public boolean removeAll(Collection c) { + return false; + } + + @Override + public boolean retainAll(Collection c) { + return false; + } + + @Override + public void clear() { + + } + + @Override + public E get(int index) { + return null; + } + + @Override + public E set(int index, E element) { + return null; + } + + @Override + public void add(int index, E element) { + + } + + @Override + public E remove(int index) { + return null; + } + + @Override + public int indexOf(Object o) { + return 0; + } + + @Override + public int lastIndexOf(Object o) { + return 0; + } + + @Override + public ListIterator listIterator() { + return null; + } + + @Override + public ListIterator listIterator(int index) { + return null; + } + + @Override + public List subList(int fromIndex, int toIndex) { + return List.of(); + } + } +} diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java new file mode 100644 index 0000000000000..db21db3bc5f0c --- /dev/null +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java @@ -0,0 +1,62 @@ +/* + * 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; + +import org.elasticsearch.xpack.esql.core.expression.Expression; +import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; + +import java.util.LinkedList; +import java.util.List; +import java.util.Queue; +import java.util.Stack; + +/** + * This validates if the actual plan contains the expected plan anywhere within the tree + */ +public class PlanContainsMatcher extends TypeSafeMatcher { + private final LogicalPlan expected; + + public static PlanContainsMatcher containsPlan(PlanBuilder expected) { + return new PlanContainsMatcher(expected.build()); + } + + public static PlanContainsMatcher containsPlan(LogicalPlan expected) { + return new PlanContainsMatcher(expected); + } + + PlanContainsMatcher(LogicalPlan expected) { + this.expected = expected; + } + + @Override + protected boolean matchesSafely(LogicalPlan actual) { + return matches(expected, actual); + } + + @Override + public void describeTo(Description description) { + + } + + private static List findMatchingChildren(LogicalPlan expected, LogicalPlan actual) { + return actual.collect(p -> p.getClass() == expected.getClass()); + } + + private static boolean matches(LogicalPlan expected, LogicalPlan initialActual) { + List candidates = findMatchingChildren(expected, initialActual); + if (candidates.isEmpty()) { + return false; + } + + // TODO: Deal with multiple matching children case + LogicalPlan actual = candidates.get(0); + return PartialPlanMatcher.matches(expected, actual); + } +} diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index dd48cfac29ea0..701a59f60c863 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -19,6 +19,8 @@ import org.elasticsearch.dissect.DissectParser; import org.elasticsearch.index.IndexMode; import org.elasticsearch.xpack.esql.EsqlTestUtils; +import org.elasticsearch.xpack.esql.PlanBuilder; +import org.elasticsearch.xpack.esql.PlanContainsMatcher; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; import org.elasticsearch.xpack.esql.analysis.Analyzer; @@ -133,6 +135,7 @@ import org.elasticsearch.xpack.esql.plan.logical.local.EmptyLocalSupplier; import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject; import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; +import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; import org.elasticsearch.xpack.esql.rule.RuleExecutor; import java.time.Duration; @@ -167,6 +170,8 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.referenceAttribute; import static org.elasticsearch.xpack.esql.EsqlTestUtils.singleValue; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; +import static org.elasticsearch.xpack.esql.PartialPlanMatcher.matchesPartialPlan; +import static org.elasticsearch.xpack.esql.PlanContainsMatcher.containsPlan; import static org.elasticsearch.xpack.esql.analysis.Analyzer.NO_FIELDS; import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.analyze; import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL; @@ -190,7 +195,6 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.emptyArray; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.hasItem; @@ -212,9 +216,7 @@ public void testEmptyProjections() { | drop salary """); - var relation = as(plan, LocalRelation.class); - assertThat(relation.output(), is(empty())); - assertThat(relation.supplier().get(), emptyArray()); + assertThat(plan, matchesPartialPlan(PlanBuilder.localRelation(List.of(), EmptyLocalSupplier.EMPTY))); } public void testEmptyProjectionInStat() { @@ -224,9 +226,7 @@ public void testEmptyProjectionInStat() { | drop c """); - var relation = as(plan, LocalRelation.class); - assertThat(relation.output(), is(empty())); - assertThat(relation.supplier().get(), emptyArray()); + assertThat(plan, matchesPartialPlan(PlanBuilder.localRelation(List.of(), EmptyLocalSupplier.EMPTY))); } /** @@ -247,7 +247,6 @@ public void testEmptyProjectInStatWithEval() { | eval x = 1, c2 = c*2 | drop c, c2 """); - var project = as(plan, Project.class); var eval = as(project.child(), Eval.class); var limit = as(eval.child(), Limit.class); From 4264ae6c9cce795a365a27703c516a1bc957493f Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 6 Aug 2025 21:15:26 +0000 Subject: [PATCH 2/3] [CI] Auto commit changes from spotless --- .../org/elasticsearch/xpack/esql/PlanContainsMatcher.java | 4 ---- .../xpack/esql/optimizer/LogicalPlanOptimizerTests.java | 3 --- 2 files changed, 7 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java index db21db3bc5f0c..a706bef1ebe94 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java @@ -7,15 +7,11 @@ package org.elasticsearch.xpack.esql; -import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.hamcrest.Description; import org.hamcrest.TypeSafeMatcher; -import java.util.LinkedList; import java.util.List; -import java.util.Queue; -import java.util.Stack; /** * This validates if the actual plan contains the expected plan anywhere within the tree diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 701a59f60c863..a4edf06bdae73 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -20,7 +20,6 @@ import org.elasticsearch.index.IndexMode; import org.elasticsearch.xpack.esql.EsqlTestUtils; import org.elasticsearch.xpack.esql.PlanBuilder; -import org.elasticsearch.xpack.esql.PlanContainsMatcher; import org.elasticsearch.xpack.esql.VerificationException; import org.elasticsearch.xpack.esql.action.EsqlCapabilities; import org.elasticsearch.xpack.esql.analysis.Analyzer; @@ -135,7 +134,6 @@ import org.elasticsearch.xpack.esql.plan.logical.local.EmptyLocalSupplier; import org.elasticsearch.xpack.esql.plan.logical.local.EsqlProject; import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation; -import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier; import org.elasticsearch.xpack.esql.rule.RuleExecutor; import java.time.Duration; @@ -171,7 +169,6 @@ import static org.elasticsearch.xpack.esql.EsqlTestUtils.singleValue; import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning; import static org.elasticsearch.xpack.esql.PartialPlanMatcher.matchesPartialPlan; -import static org.elasticsearch.xpack.esql.PlanContainsMatcher.containsPlan; import static org.elasticsearch.xpack.esql.analysis.Analyzer.NO_FIELDS; import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.analyze; import static org.elasticsearch.xpack.esql.core.expression.Literal.NULL; From 7e45bd3d58ae85e6f04e8359a554d096d6711b4c Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Mon, 11 Aug 2025 10:03:09 -0400 Subject: [PATCH 3/3] attempt at useful failure messages --- .../xpack/esql/PartialPlanMatcher.java | 63 +++++++++---------- .../xpack/esql/PlanContainsMatcher.java | 2 +- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java index 134e975d36cbe..1820007721d34 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PartialPlanMatcher.java @@ -34,63 +34,62 @@ public static PartialPlanMatcher matchesPartialPlan(LogicalPlan expected) { @Override protected boolean matchesSafely(LogicalPlan actual) { - return matches(expected, actual); + return matches(expected, actual) == null; } @Override public void describeTo(Description description) { + description.appendText(expected.toString()); + } + @Override + protected void describeMismatchSafely(LogicalPlan item, Description mismatchDescription) { + super.describeMismatchSafely(item, mismatchDescription); + mismatchDescription.appendText(System.lineSeparator()).appendText(matches(expected, item)); } - static boolean matches(LogicalPlan expected, LogicalPlan actual) { + static String matches(LogicalPlan expected, LogicalPlan actual) { if (expected.getClass() != actual.getClass()) { - assert false : "Mismatch in plan types: Expected [" + expected.getClass() + "], found [" + actual.getClass() + "]"; - return false; + return "Mismatch in plan types: Expected [" + expected.getClass() + "], found [" + actual.getClass() + "]"; } - List leftProperties = expected.nodeProperties(); - List rightProperties = actual.nodeProperties(); + List expectedProperties = expected.nodeProperties(); + List actualProperties = actual.nodeProperties(); - for (int i = 0; i < leftProperties.size(); i++) { - Object leftProperty = leftProperties.get(i); - Object rightProperty = rightProperties.get(i); + for (int i = 0; i < expectedProperties.size(); i++) { + Object expectedProperty = expectedProperties.get(i); + Object actualProperty = actualProperties.get(i); - // Only check equality if left is not null. This way we can be lenient by simply + // Only check equality if expected is not null. This way we can be lenient by simply // leaving out properties. - if (leftProperty == null || leftProperty instanceof PlanBuilder.Ignore) { + if (expectedProperty == null || expectedProperty instanceof PlanBuilder.Ignore) { continue; } - if (rightProperty == null) { - assert false : "Expected [" + leftProperty.getClass() + "], found null."; - return false; + if (actualProperty == null) { + return "Expected [" + expectedProperty.getClass() + "], found null."; } - if (leftProperty instanceof LogicalPlan l) { - if (rightProperty instanceof LogicalPlan rightPlan) { - if (matches(l, rightPlan) == false) { - return false; + if (expectedProperty instanceof LogicalPlan l) { + if (actualProperty instanceof LogicalPlan rightPlan) { + String subMatch = matches(l, rightPlan); + if (subMatch != null) { + return subMatch; } - } else { - assert false - : "Mismatch in types: Expected [" + leftProperty.getClass() + "], found [" + rightProperty.getClass() + "]"; - return false; - } - } else if (leftProperty instanceof Expression e) { - if (rightProperty instanceof Expression rightExpr) { + } else {} + } else if (expectedProperty instanceof Expression e) { + if (actualProperty instanceof Expression rightExpr) { if (e.semanticEquals(rightExpr) == false) { - assert false : "Mismatch in expressions: Expected [" + e + "], found [" + rightExpr + "]"; - return false; + return "Mismatch in expressions: Expected [" + e + "], found [" + rightExpr + "]"; } } else { - return false; + return "Mismatch in types: Expected [" + expectedProperty.getClass() + "], found [" + actualProperty.getClass() + "]"; } - } else if (leftProperty.equals(rightProperty) == false) { - assert false : "Mismatch in properties: Expected [" + leftProperty + "], found [" + rightProperty + "]"; - return false; + } else if (expectedProperty.equals(actualProperty) == false) { + return "Mismatch in properties: Expected [" + expectedProperty + "], found [" + actualProperty + "]"; } } - return true; + return null; } } diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java index a706bef1ebe94..5b04b026460d2 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/PlanContainsMatcher.java @@ -53,6 +53,6 @@ private static boolean matches(LogicalPlan expected, LogicalPlan initialActual) // TODO: Deal with multiple matching children case LogicalPlan actual = candidates.get(0); - return PartialPlanMatcher.matches(expected, actual); + return PartialPlanMatcher.matches(expected, actual) == null; } }