Check that all elements on each disjunction side have full text functions

Author: carlosdelest

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java

@@ -240,7 +240,7 @@ public void testWhereMatchWithFunctions() {
             error.getMessage(),
             containsString(
                 "Invalid condition [content:\"fox\" OR to_upper(content) == \"FOX\"]. "
-                    + "[:] operator can't be used as part of an or condition"
+                    + "[:] operator can be used in an OR condition, but only if just full text functions are used in the OR condition"
             )
         );
     }

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchOperatorIT.java

@@ -215,8 +215,7 @@ public void testWhereMatchWithFunctions() {
         assertThat(
             error.getMessage(),
             containsString(
-                "Invalid condition [content:\"fox\" OR to_upper(content) == \"FOX\"]. "
-                    + "[:] operator can't be used as part of an or condition"
+                "Invalid condition [content:\"fox\" OR to_upper(content) == \"FOX\"]. " + "[:] operator can't be used in an OR condition"
             )
         );
     }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

@@ -768,9 +768,11 @@ private static void checkRemoteEnrich(LogicalPlan plan, Set<Failure> failures) {
     }
 
     /**
-     * Checks whether a condition contains a disjunction with the specified typeToken. Adds to failure if it does.
+     * Checks whether a condition contains a disjunction with a full text search.
+     * If it does, check that every element of the disjunction is a full text search or combinations (AND, OR, NOT) of them.
+     * If not, add a failure to the failures collection.
      *
-     * @param condition        condition to check for disjunctions
+     * @param condition        condition to check for disjunctions of full text searches
      * @param typeNameProvider provider for the type name to add in the failure message
      * @param failures         failures collection to add to
      */
@@ -779,29 +781,56 @@ private static void checkFullTextSearchDisjunctions(
         java.util.function.Function<FullTextFunction, String> typeNameProvider,
         Set<Failure> failures
     ) {
-        condition.forEachUp(Or.class, or -> {
-            boolean left = onlyFullTextFunctionsInExpression(or.left());
-            boolean right = onlyFullTextFunctionsInExpression(or.right());
-            if (left ^ right) {
-                Holder<String> elementName = new Holder<>();
-                if (left) {
-                    or.left().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
-                } else {
-                    or.right().forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
-                }
-                failures.add(
-                    fail(
-                        or,
-                        "Invalid condition [{}]. {} can be used as part of an OR condition, "
-                            + "but only if other full text functions are used as part of the condition",
-                        or.sourceText(),
-                        elementName.get()
-                    )
-                );
+        int failuresCount = failures.size();
+        condition.forEachDown(Or.class, or -> {
+            if (failures.size() > failuresCount) {
+                // Exit early if we already have a failures
+                return;
+            }
+            Expression left = or.left();
+            boolean leftHasFullText = left.anyMatch(FullTextFunction.class::isInstance);
+            boolean leftHasOnlyFullText = onlyFullTextFunctionsInExpression(left);
+            Expression right = or.right();
+            boolean rightHasFullText = right.anyMatch(FullTextFunction.class::isInstance);
+            boolean rightHasOnlyFullText = onlyFullTextFunctionsInExpression(right);
+
+            if (leftHasFullText && leftHasOnlyFullText == false) {
+                disjunctionFailure(or, left, typeNameProvider, failures);
+            } else if (rightHasFullText && rightHasOnlyFullText == false) {
+                disjunctionFailure(or, right, typeNameProvider, failures);
+            } else if (leftHasFullText ^ rightHasFullText) {
+                disjunctionFailure(or, leftHasFullText ? left : right, typeNameProvider, failures);
             }
         });
     }
 
+    /**
+     * Add a disjunction failure to the failures collection.
+     * @param parentExpression parent expression to include in the failure
+     * @param expressionWithError expression that provoked the failure
+     * @param typeNameProvider provider for the type name to add in the failure message
+     * @param failures failures collection to add to
+     */
+    private static void disjunctionFailure(
+        Expression parentExpression,
+        Expression expressionWithError,
+        java.util.function.Function<FullTextFunction, String> typeNameProvider,
+        Set<Failure> failures
+    ) {
+        Holder<String> elementName = new Holder<>();
+        // Get first function name to add to the failure message
+        expressionWithError.forEachDown(FullTextFunction.class, ftf -> elementName.set(typeNameProvider.apply(ftf)));
+        failures.add(
+            fail(
+                parentExpression,
+                "Invalid condition [{}]. {} can be used in an OR condition, "
+                    + "but only if just full text functions are used in the OR condition",
+                parentExpression.sourceText(),
+                elementName.get()
+            )
+        );
+    }
+
     /**
      * Checks whether an expression contains just full text functions or negations (NOT) and combinations (AND, OR) of full text functions
      *
@@ -811,15 +840,32 @@ private static void checkFullTextSearchDisjunctions(
     private static boolean onlyFullTextFunctionsInExpression(Expression expression) {
         if (expression instanceof FullTextFunction) {
             return true;
-        } else if (expression instanceof Not || expression instanceof BinaryLogic) {
-            for (Expression child : expression.children()) {
-                if (onlyFullTextFunctionsInExpression(child) == false) {
-                    return false;
-                }
-            }
+        } else if (expression instanceof Not) {
+            return onlyFullTextFunctionsInExpression(expression.children().get(0));
+        } else if (expression instanceof BinaryLogic binaryLogic) {
+            return onlyFullTextFunctionsInExpression(binaryLogic.left()) && onlyFullTextFunctionsInExpression(binaryLogic.right());
+        }
+
+        return false;
+    }
+
+    /**
+     * Checks whether an expression contains a full text function as part of it
+     *
+     * @param expression expression to check
+     * @return true if the expression or any of its children is a full text function, false otherwise
+     */
+    private static boolean anyFullTextFunctionsInExpression(Expression expression) {
+        if (expression instanceof FullTextFunction) {
             return true;
         }
 
+        for (Expression child : expression.children()) {
+            if (anyFullTextFunctionsInExpression(child)) {
+                return true;
+            }
+        }
+
         return false;
     }
 

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@@ -1168,14 +1168,14 @@ public void testMatchInsideEval() throws Exception {
     public void testMatchFilter() throws Exception {
         assertEquals(
             "1:19: Invalid condition [first_name:\"Anna\" or starts_with(first_name, \"Anne\")]. "
-                + "[:] operator can be used as part of an OR condition, "
-                + "but only if other full text functions are used as part of the condition",
+                + "[:] operator 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]. [:] operator can be used as part of an OR "
-                + "condition, but only if other full text functions are used as part of the condition",
+            "1:51: Invalid condition [first_name:\"Anna\" OR new_salary > 100]. [:] operator 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")
         );
     }
@@ -1413,45 +1413,26 @@ 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,
-                "1:19: Invalid condition [{} or length(first_name) > 12]. "
-                    + "[{}] "
-                    + functionType
-                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
-                functionInvocation,
-                functionName
-            ),
-            error("from test | where " + functionInvocation + " or length(first_name) > 12")
-        );
-        assertEquals(
-            LoggerMessageFormat.format(
-                null,
-                "1:20: Invalid condition [{} or first_name is not null]. "
-                    + "[{}] "
-                    + functionType
-                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
-                functionInvocation,
-                functionName
-            ),
-            error(
-                "from test | where ("
-                    + functionInvocation
-                    + " or first_name is not null) or (length(first_name) > 12 and match(last_name, \"Smith\"))"
-            )
-        );
-        assertEquals(
-            LoggerMessageFormat.format(
-                null,
-                "1:19: Invalid condition [{} or (last_name is not null and first_name is null)]. "
-                    + "[{}] "
-                    + functionType
-                    + " can be used as part of an OR condition, but only if other full text functions are used as part of the condition",
-                functionInvocation,
-                functionName
+                "{}: Invalid condition [{}]. [{}] {} can be used in an OR condition, "
+                    + "but only if just full text functions are used in the OR condition",
+                position,
+                expression,
+                functionName,
+                functionType
             ),
-            error("from test | where " + functionInvocation + " or (last_name is not null and first_name is null)")
+            error("from test | where " + expression)
         );
     }
 
@@ -1465,35 +1446,23 @@ 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);
+
         passes("from test | where " + functionInvocation + " or match(first_name, \"Anna\")");
         passes("from test | where " + functionInvocation + " or not match(first_name, \"Anna\")");
         passes("from test | where (" + functionInvocation + " or match(first_name, \"Anna\")) and length(first_name) > 10");
         passes("from test | where (" + functionInvocation + " or match(first_name, \"Anna\")) and match(last_name, \"Smith\")");
         passes("from test | where " + functionInvocation + " or (match(first_name, \"Anna\") and match(last_name, \"Smith\"))");
-
-        assertEquals(
-            LoggerMessageFormat.format(
-                null,
-                "1:19: Invalid condition [{} or length(first_name) > 10]. [{}] {} can be used as part of an OR condition, "
-                    + "but only if other full text functions are used as part of the condition",
-                functionInvocation,
-                functionName,
-                functionType
-            ),
-            error("from test | where " + functionInvocation + " or length(first_name) > 10")
-        );
-        assertEquals(
-            LoggerMessageFormat.format(
-                null,
-                "1:19: Invalid condition [{} or (match(last_name, \"Anneke\") and length(first_name) > 10)]."
-                    + " [{}] {} can be used as part of an OR condition, "
-                    + "but only if other full text functions are used as part of the condition",
-                functionInvocation,
-                functionName,
-                functionType
-            ),
-            error("from test | where " + functionInvocation + " or (match(last_name, \"Anneke\") and length(first_name) > 10)")
-        );
     }
 
     public void testQueryStringFunctionWithNonBooleanFunctions() {

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/180_match_operator.yml

@@ -170,7 +170,7 @@ setup:
   - match: { error.reason: "Found 1 problem\nline 1:36: Unknown column [content], did you mean [count(*)]?" }
 
 ---
-"match with functions":
+"match with disjunctions":
   - do:
       catch: bad_request
       allowed_warnings_regex:
@@ -183,6 +183,19 @@ setup:
   - match: { error.type: verification_exception }
   - match: { error.reason: "/.+Invalid\\ condition\\ \\[content\\:\"fox\"\\ OR\\ to_upper\\(content\\)\\ ==\\ \"FOX\"\\]\\.\\ \\[\\:\\]\\ operator.+/" }
 
+  - do:
+      catch: bad_request
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'FROM test | WHERE content:"fox" OR to_upper(content) == "FOX"'
+
+  - match: { status: 400 }
+  - match: { error.type: verification_exception }
+  - match: { error.reason: "/.+Invalid\\ condition\\ \\[content\\:\"fox\"\\ OR\\ to_upper\\(content\\)\\ ==\\ \"FOX\"\\]\\.\\ \\[\\:\\]\\ operator.+/" }
+
+
 ---
 "match within eval":
   - do: