Move some checks from Or to FullTextFunction

Author: carlosdelest

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java

@@ -248,38 +248,46 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Failures failu
      * @param condition        condition to check for disjunctions of full text searches
      * @param failures         failures collection to add to
      */
-    public static void checkFullTextSearchDisjunctions(Expression condition, Failures failures) {
+    private static void checkFullTextSearchDisjunctions(Expression condition, Failures failures) {
         Holder<Boolean> isInvalid = new Holder<>(false);
         condition.forEachDown(Or.class, or -> {
             if (isInvalid.get()) {
                 // Exit early if we already have a failures
                 return;
             }
-            boolean hasFullText = or.anyMatch(FullTextFunction.class::isInstance);
-            if (hasFullText) {
-                boolean hasOnlyFullText = onlyFullTextFunctionsInExpression(or);
-                if (hasOnlyFullText == false) {
-                    isInvalid.set(true);
-                    failures.add(
-                        fail(
-                            or,
-                            "Invalid condition when using METADATA _score [{}]. 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()
-                        )
-                    );
-                }
+            if (checkDisjunctionPushable(or) == false) {
+                isInvalid.set(true);
+                failures.add(
+                    fail(
+                        or,
+                        "Invalid condition when using METADATA _score [{}]. 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()
+                    )
+                );
             }
         });
     }
 
+    /**
+     * Checks if a disjunction is pushable from the point of view of FullTextFunctions. Either it has no FullTextFunctions or
+     * all it contains are FullTextFunctions.
+     *
+     * @param or disjunction to check
+     * @return true if the disjunction is pushable, false otherwise
+     */
+    public static boolean checkDisjunctionPushable(Or or) {
+        boolean hasFullText = or.anyMatch(FullTextFunction.class::isInstance);
+        return hasFullText == false || onlyFullTextFunctionsInExpression(or);
+    }
+
     /**
      * Checks whether an expression contains just full text functions or negations (NOT) and combinations (AND, OR) of full text functions
      *
      * @param expression expression to check
      * @return true if all children are full text functions or negations of full text functions, false otherwise
      */
-    private static boolean onlyFullTextFunctionsInExpression(Expression expression) {
+    public static boolean onlyFullTextFunctionsInExpression(Expression expression) {
         if (expression instanceof FullTextFunction) {
             return true;
         } else if (expression instanceof Not) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/logical/Or.java

@@ -8,24 +8,17 @@
 
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
-import org.elasticsearch.xpack.esql.common.Failures;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
-import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
 import org.elasticsearch.xpack.esql.core.expression.predicate.Negatable;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.expression.function.fulltext.FullTextFunction;
 import org.elasticsearch.xpack.esql.expression.predicate.Predicates;
 import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
-import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 
 import java.io.IOException;
-import java.util.function.BiConsumer;
 
-import static org.elasticsearch.xpack.esql.common.Failure.fail;
-
-public class Or extends BinaryLogic implements Negatable<BinaryLogic>, PostAnalysisPlanVerificationAware {
+public class Or extends BinaryLogic implements Negatable<BinaryLogic> {
     public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(Expression.class, "Or", Or::new);
 
     public Or(Source source, Expression left, Expression right) {
@@ -69,48 +62,6 @@ protected Expression canonicalize() {
 
     @Override
     public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
-        return super.translatable(pushdownPredicates) && checkPushableFullTextSearchFunctions();
-    }
-
-    @Override
-    public BiConsumer<LogicalPlan, Failures> postAnalysisPlanVerification() {
-        return (plan, failures) -> {
-            boolean usesScore = plan.output()
-                .stream()
-                .anyMatch(attr -> attr instanceof MetadataAttribute ma && ma.name().equals(MetadataAttribute.SCORE));
-            if (usesScore && checkPushableFullTextSearchFunctions() == false) {
-                failures.add(
-                    fail(
-                        this,
-                        "Invalid condition when using METADATA _score [{}]. Full text functions can be used in an OR condition, "
-                            + "but only if just full text functions are used in the OR condition",
-                        sourceText()
-                    )
-                );
-            }
-        };
-    }
-
-    private boolean checkPushableFullTextSearchFunctions() {
-        boolean hasFullText = anyMatch(FullTextFunction.class::isInstance);
-        return hasFullText == false || onlyFullTextFunctionsInExpression(this);
-    }
-
-    /**
-     * Checks whether an expression contains just full text functions or negations (NOT) and combinations (AND, OR) of full text functions
-     *
-     * @param expression expression to check
-     * @return true if all children are full text functions or negations of full text functions, false otherwise
-     */
-    private static boolean onlyFullTextFunctionsInExpression(Expression expression) {
-        if (expression instanceof FullTextFunction) {
-            return true;
-        } 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;
+        return super.translatable(pushdownPredicates) && FullTextFunction.checkDisjunctionPushable(this);
     }
 }

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java

@@ -38,14 +38,14 @@
 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.logical.Or;
 import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThan;
 import org.elasticsearch.xpack.esql.index.EsIndex;
 import org.elasticsearch.xpack.esql.index.IndexResolution;