SQL: Small code improvements of Pipes & Processors (#40909)

- Remove superfluous methods that are already
defined in superclasses.
- Improve tests for null folding on conditionals

Author: matriv

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/pipeline/Pipe.java

@@ -12,8 +12,8 @@
 import org.elasticsearch.xpack.sql.expression.Attribute;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.Node;
+import org.elasticsearch.xpack.sql.tree.Source;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -53,13 +53,7 @@ public void collectFields(SqlSourceBuilder sourceBuilder) {
 
     @Override
     public boolean supportedByAggsOnlyQuery() {
-        for (Pipe pipe : children()) {
-            if (pipe.supportedByAggsOnlyQuery()) {
-                return true;
-            }
-        }
-
-        return false;
+        return children().stream().anyMatch(Pipe::supportedByAggsOnlyQuery);
     }
 
     public abstract Processor asProcessor();
@@ -83,4 +77,4 @@ public Pipe resolveAttributes(AttributeResolver resolver) {
     public interface AttributeResolver {
         FieldExtraction resolve(Attribute attribute);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java

@@ -8,13 +8,11 @@
 
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
-import org.elasticsearch.xpack.sql.expression.Nullability;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
 import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
-import org.elasticsearch.xpack.sql.type.DataType;
+import org.elasticsearch.xpack.sql.tree.Source;
 
 import java.util.Arrays;
 import java.util.List;
@@ -47,21 +45,6 @@ protected TypeResolution resolveType() {
         return TypeResolution.TYPE_RESOLVED;
     }
 
-    @Override
-    public DataType dataType() {
-        return dataType;
-    }
-
-    @Override
-    public boolean foldable() {
-        return Expressions.foldable(children());
-    }
-
-    @Override
-    public Nullability nullable() {
-        return Nullability.UNKNOWN;
-    }
-
     @Override
     public Object fold() {
         return NullIfProcessor.apply(children().get(0).fold(), children().get(1).fold());

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIfProcessor.java

@@ -59,8 +59,12 @@ public static Object apply(Object leftValue, Object rightValue) {
 
     @Override
     public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
         NullIfProcessor that = (NullIfProcessor) o;
         return Objects.equals(leftProcessor, that.leftProcessor) &&
             Objects.equals(rightProcessor, that.rightProcessor);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/InPipe.java

@@ -5,26 +5,20 @@
  */
 package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
 
-import org.elasticsearch.xpack.sql.capabilities.Resolvables;
-import org.elasticsearch.xpack.sql.execution.search.FieldExtraction;
-import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder;
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.gen.pipeline.MultiPipe;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
-import org.elasticsearch.xpack.sql.tree.Source;
+import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.stream.Collectors;
 
-public class InPipe extends Pipe {
-
-    private List<Pipe> pipes;
+public class InPipe extends MultiPipe {
 
     public InPipe(Source source, Expression expression, List<Pipe> pipes) {
         super(source, expression, pipes);
-        this.pipes = pipes;
     }
 
     @Override
@@ -37,36 +31,12 @@ public final Pipe replaceChildren(List<Pipe> newChildren) {
 
     @Override
     protected NodeInfo<InPipe> info() {
-        return NodeInfo.create(this, InPipe::new, expression(), pipes);
-    }
-
-    @Override
-    public boolean supportedByAggsOnlyQuery() {
-        return pipes.stream().allMatch(FieldExtraction::supportedByAggsOnlyQuery);
-    }
-
-    @Override
-    public final Pipe resolveAttributes(AttributeResolver resolver) {
-        List<Pipe> newPipes = new ArrayList<>(pipes.size());
-        for (Pipe p : pipes) {
-            newPipes.add(p.resolveAttributes(resolver));
-        }
-        return replaceChildren(newPipes);
-    }
-
-    @Override
-    public boolean resolved() {
-        return Resolvables.resolved(pipes);
-    }
-
-    @Override
-    public final void collectFields(SqlSourceBuilder sourceBuilder) {
-        pipes.forEach(p -> p.collectFields(sourceBuilder));
+        return NodeInfo.create(this, InPipe::new, expression(), children());
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(pipes);
+        return Objects.hash(children());
     }
 
     @Override
@@ -80,11 +50,11 @@ public boolean equals(Object obj) {
         }
 
         InPipe other = (InPipe) obj;
-        return Objects.equals(pipes, other.pipes);
+        return Objects.equals(children(), other.children());
     }
 
     @Override
-    public InProcessor asProcessor() {
-        return new InProcessor(pipes.stream().map(Pipe::asProcessor).collect(Collectors.toList()));
+    public InProcessor asProcessor(List<Processor> processors) {
+        return new InProcessor(processors);
     }
 }

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

@@ -43,7 +43,9 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.string.Repeat;
 import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator;
 import org.elasticsearch.xpack.sql.expression.predicate.Range;
+import org.elasticsearch.xpack.sql.expression.predicate.conditional.ArbitraryConditionalFunction;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce;
+import org.elasticsearch.xpack.sql.expression.predicate.conditional.ConditionalFunction;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.Least;
@@ -97,6 +99,7 @@
 import org.elasticsearch.xpack.sql.util.CollectionUtils;
 import org.elasticsearch.xpack.sql.util.StringUtils;
 
+import java.lang.reflect.Constructor;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -445,18 +448,32 @@ public void testNullFoldingDoesNotApplyOnLogicalExpressions() {
         assertEquals(and, rule.rule(and));
     }
 
-    public void testNullFoldingDoesNotApplyOnConditionals() {
+    @SuppressWarnings("unchecked")
+    public void testNullFoldingDoesNotApplyOnConditionals() throws Exception {
         FoldNull rule = new FoldNull();
 
-        Coalesce coalesce = new Coalesce(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO));
-        assertEquals(coalesce, rule.rule(coalesce));
-        coalesce = new Coalesce(EMPTY, Arrays.asList(Literal.NULL, NULL, NULL));
-        assertEquals(coalesce, rule.rule(coalesce));
+        Class<ConditionalFunction> clazz =
+            (Class<ConditionalFunction>) randomFrom(IfNull.class, NullIf.class);
+        Constructor<ConditionalFunction> ctor = clazz.getConstructor(Source.class, Expression.class, Expression.class);
+        ConditionalFunction conditionalFunction = ctor.newInstance(EMPTY, Literal.NULL, ONE);
+        assertEquals(conditionalFunction, rule.rule(conditionalFunction));
+        conditionalFunction = ctor.newInstance(EMPTY, ONE, Literal.NULL);
+        assertEquals(conditionalFunction, rule.rule(conditionalFunction));
+        conditionalFunction = ctor.newInstance(EMPTY, Literal.NULL, Literal.NULL);
+        assertEquals(conditionalFunction, rule.rule(conditionalFunction));
+    }
+
+    @SuppressWarnings("unchecked")
+    public void testNullFoldingDoesNotApplyOnArbitraryConditionals() throws Exception {
+        FoldNull rule = new FoldNull();
 
-        Greatest greatest = new Greatest(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO));
-        assertEquals(greatest, rule.rule(greatest));
-        greatest = new Greatest(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO));
-        assertEquals(greatest, rule.rule(greatest));
+        Class<ArbitraryConditionalFunction> clazz =
+            (Class<ArbitraryConditionalFunction>) randomFrom(Coalesce.class, Greatest.class, Least.class);
+        Constructor<ArbitraryConditionalFunction> ctor = clazz.getConstructor(Source.class, List.class);
+        ArbitraryConditionalFunction conditionalFunction = ctor.newInstance(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO));
+        assertEquals(conditionalFunction, rule.rule(conditionalFunction));
+        conditionalFunction = ctor.newInstance(EMPTY, Arrays.asList(Literal.NULL, NULL, NULL));
+        assertEquals(conditionalFunction, rule.rule(conditionalFunction));
     }
 
     public void testSimplifyCoalesceNulls() {