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

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

(cherry picked from commit 67f9404f5004362e569353d1e950ffe5d7a9ab6e)
This commit is contained in:
Marios Trivyzas 2019-04-08 10:13:15 +02:00
parent c29027d99e
commit ddf17dfb1e
No known key found for this signature in database
GPG Key ID: 8817B46B0CF36A3F
5 changed files with 45 additions and 77 deletions

View File

@ -12,8 +12,8 @@ import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder;
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 abstract class Pipe extends Node<Pipe> implements FieldExtraction, Resolv
@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();

View File

@ -8,13 +8,11 @@ package org.elasticsearch.xpack.sql.expression.predicate.conditional;
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 @@ public class NullIf extends ConditionalFunction {
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());

View File

@ -59,8 +59,12 @@ public class NullIfProcessor implements Processor {
@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);

View File

@ -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 class InPipe extends Pipe {
@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 class InPipe extends Pipe {
}
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);
}
}

View File

@ -43,7 +43,9 @@ import org.elasticsearch.xpack.sql.expression.function.scalar.string.Concat;
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.type.EsField;
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 class OptimizerTests extends ESTestCase {
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));
}
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));
@SuppressWarnings("unchecked")
public void testNullFoldingDoesNotApplyOnArbitraryConditionals() throws Exception {
FoldNull rule = new FoldNull();
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() {