Fix tests

carlosdelest · Jan 16, 2025 · 318b06e · 318b06e
1 parent ed62f9f
commit 318b06e
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 48 deletions.
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@@ -8,7 +8,6 @@
 package org.elasticsearch.xpack.esql.analysis;
 
 import org.elasticsearch.Build;
-import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.esql.VerificationException;
 import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
@@ -1190,21 +1189,6 @@ public void testMatchInsideEval() throws Exception {
         );
     }
 
-    public void testMatchFilter() throws Exception {
-        assertEquals(
-            "1:19: Invalid condition [first_name:\"Anna\" or starts_with(first_name, \"Anne\")]. "
-                + "Full text functions can be used in an OR condition, "
-                + "but only if just full text functions are used in the OR condition",
-            error("from test | where first_name:\"Anna\" or starts_with(first_name, \"Anne\")")
-        );
-
-        assertEquals(
-            "1:51: Invalid condition [first_name:\"Anna\" OR new_salary > 100]. Full text functions can be"
-                + " used in an OR condition, but only if just full text functions are used in the OR condition",
-            error("from test | eval new_salary = salary + 10 | where first_name:\"Anna\" OR new_salary > 100")
-        );
-    }
-
     public void testMatchFunctionNotAllowedAfterCommands() throws Exception {
         assertEquals(
             "1:24: [MATCH] function cannot be used after LIMIT",
@@ -1426,25 +1410,13 @@ public void testMatchOperatorWithDisjunctions() {
     }
 
     private void checkWithDisjunctions(String functionName, String functionInvocation, String functionType) {
-        String expression = functionInvocation + " or length(first_name) > 12";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-        expression = "(" + functionInvocation + " or first_name is not null) or (length(first_name) > 12 and match(last_name, \"Smith\"))";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-        expression = functionInvocation + " or (last_name is not null and first_name is null)";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-    }
-
-    private void checkdisjunctionError(String position, String expression, String functionName, String functionType) {
-        assertEquals(
-            LoggerMessageFormat.format(
-                null,
-                "{}: Invalid condition [{}]. Full text functions can be used in an OR condition, "
-                    + "but only if just full text functions are used in the OR condition",
-                position,
-                expression
-            ),
-            error("from test | where " + expression)
+        query("from test | where " + functionInvocation + " or length(first_name) > 12");
+        query(
+            "from test | where ("
+                + functionInvocation
+                + " or first_name is not null) or (length(first_name) > 12 and match(last_name, \"Smith\"))"
         );
+        query("from test | where " + functionInvocation + " or (last_name is not null and first_name is null)");
     }
 
     public void testFullTextFunctionsDisjunctions() {
@@ -1456,17 +1428,13 @@ public void testFullTextFunctionsDisjunctions() {
 
     private void checkWithFullTextFunctionsDisjunctions(String functionName, String functionInvocation, String functionType) {
 
-        String expression = functionInvocation + " or length(first_name) > 10";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-
-        expression = "match(last_name, \"Anneke\") or (" + functionInvocation + " and length(first_name) > 10)";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-
-        expression = "("
-            + functionInvocation
-            + " and length(first_name) > 0) or (match(last_name, \"Anneke\") and length(first_name) > 10)";
-        checkdisjunctionError("1:19", expression, functionName, functionType);
-
+        query("from test | where " + functionInvocation + " or length(first_name) > 10");
+        query("from test | where match(last_name, \"Anneke\") or (" + functionInvocation + " and length(first_name) > 10)");
+        query(
+            "from test | where ("
+                + functionInvocation
+                + " and length(first_name) > 0) or (match(last_name, \"Anneke\") and length(first_name) > 10)"
+        );
         query("from test | where " + functionInvocation + " or match(first_name, \"Anna\")");
         query("from test | where " + functionInvocation + " or not match(first_name, \"Anna\")");
         query("from test | where (" + functionInvocation + " or match(first_name, \"Anna\")) and length(first_name) > 10");

diff --git a/...src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/...src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java
@@ -38,13 +38,15 @@
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
+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.core.type.EsField;
 import org.elasticsearch.xpack.esql.core.util.Holder;
 import org.elasticsearch.xpack.esql.enrich.ResolvedEnrichPolicy;
 import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
 import org.elasticsearch.xpack.esql.expression.function.fulltext.Match;
+import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThan;
 import org.elasticsearch.xpack.esql.index.EsIndex;
 import org.elasticsearch.xpack.esql.index.IndexResolution;
 import org.elasticsearch.xpack.esql.optimizer.rules.logical.ExtractAggregateCommonFilter;
@@ -1632,19 +1634,43 @@ public void testMatchWithFieldCasting() {
         assertThat(queryBuilder.value(), is(123456));
     }
 
-    public void testMatchFunctionWithNonPushableDisjunction() {
+    public void testMatchFunctionWithPushableConjunction() {
         String query = """
             from test
             | where match(last_name, "Smith") and length(first_name) > 10
             """;
         var plan = plannerOptimizer.plan(query);
 
+        var limit = as(plan, LimitExec.class);
+        var exchange = as(limit.child(), ExchangeExec.class);
+        var project = as(exchange.child(), ProjectExec.class);
+        var fieldExtract = as(project.child(), FieldExtractExec.class);
+        var filterLimit = as(fieldExtract.child(), LimitExec.class);
+        var filter = as(filterLimit.child(), FilterExec.class);
+        assertThat(filter.condition(), instanceOf(GreaterThan.class));
+        var fieldFilterExtract = as(filter.child(), FieldExtractExec.class);
+        var esQuery = as(fieldFilterExtract.child(), EsQueryExec.class);
+        assertThat(esQuery.query(), instanceOf(MatchQueryBuilder.class));
+    }
+
+    public void testMatchFunctionWithNonPushableDisjunction() {
+        String query = """
+            from test
+            | where match(last_name, "Smith") or length(first_name) > 10
+            """;
+        var plan = plannerOptimizer.plan(query);
+
         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 esQuery = as(field.child(), EsQueryExec.class);
-        assertThat(as(esQuery.limit(), Literal.class).value(), is(1000));
+        var filterLimit = as(field.child(), LimitExec.class);
+        var filter = as(filterLimit.child(), FilterExec.class);
+        Or or = as(filter.condition(), Or.class);
+        assertThat(or.left(), instanceOf(Match.class));
+        assertThat(or.right(), instanceOf(GreaterThan.class));
+        var fieldExtract = as(filter.child(), FieldExtractExec.class);
+        assertThat(fieldExtract.child(), instanceOf(EsQueryExec.class));
     }
 
     private QueryBuilder wrapWithSingleQuery(String query, QueryBuilder inner, String fieldName, Source source) {