Simplify disjunction algorithm

carlosdelest · Dec 17, 2024 · 8901591 · 8901591
1 parent 6449d22
commit 8901591
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 49 deletions.
diff --git a/...sql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java b/...sql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/plugin/MatchFunctionIT.java
@@ -240,7 +240,7 @@ public void testWhereMatchWithFunctions() {
             error.getMessage(),
             containsString(
                 "Invalid condition [match(content, \"fox\") OR to_upper(content) == \"FOX\"]. "
-                    + "[MATCH] function can be used in an OR condition, but only if just full text functions are used in the OR condition"
+                    + "Full text functions can be used in an OR condition, but only if just full text functions are used in the OR condition"
             )
         );
     }

diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java
@@ -27,7 +27,6 @@
 import org.elasticsearch.xpack.esql.core.expression.predicate.logical.Or;
 import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison;
 import org.elasticsearch.xpack.esql.core.type.DataType;
-import org.elasticsearch.xpack.esql.core.util.Holder;
 import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
 import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
 import org.elasticsearch.xpack.esql.expression.function.aggregate.FilteredExpression;
@@ -786,50 +785,23 @@ private static void checkFullTextSearchDisjunctions(
                 // 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);
+            boolean hasFullText = or.anyMatch(FullTextFunction.class::isInstance);
+            if (hasFullText) {
+                boolean hasOnlyFullText = onlyFullTextFunctionsInExpression(or);
+                if (hasOnlyFullText == false) {
+                    failures.add(
+                        fail(
+                            or,
+                            "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",
+                            or.sourceText()
+                        )
+                    );
+                }
             }
         });
     }
 
-    /**
-     * 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
      *

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
@@ -1166,14 +1166,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 in an OR condition, "
+                + "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]. [:] operator can be used in an OR "
-                + "condition, but only if just full text functions are used in the OR condition",
+            "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")
         );
     }
@@ -1423,12 +1423,10 @@ private void checkdisjunctionError(String position, String expression, String fu
         assertEquals(
             LoggerMessageFormat.format(
                 null,
-                "{}: Invalid condition [{}]. [{}] {} can be used in an OR condition, "
+                "{}: 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,
-                functionName,
-                functionType
+                expression
             ),
             error("from test | where " + expression)
         );