Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
costin committed Nov 5, 2018
1 parent 7225f38 commit 65605e8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 4 deletions.
17 changes: 16 additions & 1 deletion x-pack/plugin/sql/qa/src/main/resources/null.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,35 @@ null
;


divOfNull
divOfCastedNull
SELECT 5 / CAST(NULL AS FLOAT) + 10 AS n;

n:d
null
;

divNoNull
SELECT 5 / null + 1 AS n;

n:i
null
;

coalesceJustWithNull
SELECT COALESCE(null, null, null) AS c;

c
null
;

coalesceFirstNotNull
SELECT COALESCE(123) AS c;

c
123
;


coalesceWithFirstNullOfString
SELECT COALESCE(null, 'first') AS c;

Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugin/sql/qa/src/main/resources/null.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@ coalesceField
SELECT COALESCE(null, ABS(emp_no) + 1) AS c FROM test_emp ORDER BY emp_no LIMIT 5;

coalesceHaving
SELECT COALESCE(languages, 0) c FROM test_emp GROUP BY languages ORDER BY languages;
SELECT COALESCE(null, ABS(MAX(emp_no)) + 1, 123) AS c FROM test_emp GROUP BY languages HAVING c > 100 ORDER BY languages LIMIT 5;

coalesceWhere
SELECT COALESCE(null, ABS(emp_no) + 1, 123) AS c FROM test_emp WHERE COALESCE(null, ABS(emp_no) + 1, 123, 321) > 100 ORDER BY emp_no NULLS FIRST LIMIT 5;
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ static <T extends Function> FunctionDefinition def(Class<T> function,
};
return def(function, builder, true, aliases);
}

interface DatetimeUnaryFunctionBuilder<T> {
T build(Location location, Expression target, TimeZone tz);
}
Expand All @@ -397,6 +398,7 @@ static <T extends Function> FunctionDefinition def(Class<T> function,
};
return def(function, builder, false, aliases);
}

interface BinaryFunctionBuilder<T> {
T build(Location location, Expression lhs, Expression rhs);
}
Expand All @@ -415,6 +417,7 @@ private static FunctionDefinition def(Class<? extends Function> function, Functi
};
return new FunctionDefinition(primaryName, unmodifiableList(Arrays.asList(aliases)), function, datetime, realBuilder);
}

private interface FunctionBuilder {
Function build(Location location, List<Expression> children, boolean distinct, TimeZone tz);
}
Expand Down Expand Up @@ -474,6 +477,7 @@ private static <T extends Function> FunctionDefinition def(Class<T> function,
ctorRef.build(location, children.get(0), children.get(0).dataType());
return def(function, builder, false, aliases);
}

private interface CastFunctionBuilder<T> {
T build(Location location, Expression expression, DataType dataType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public ScriptTemplate asScript() {
templates.add(asScript(ex));
}

StringJoiner template = new StringJoiner(",", "{sql}.coalesce(", ")");
StringJoiner template = new StringJoiner(",", "{sql}.coalesce([", "])");
ParamsBuilder params = paramsBuilder();

for (ScriptTemplate scriptTemplate : templates) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.elasticsearch.xpack.sql.expression.function.scalar.string.Ascii;
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.operator.comparison.In;
import org.elasticsearch.xpack.sql.expression.predicate.Range;
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce;
import org.elasticsearch.xpack.sql.expression.predicate.logical.And;
Expand All @@ -50,6 +49,7 @@
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan;
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThanOrEqual;
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In;
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.sql.expression.predicate.regex.Like;
Expand Down Expand Up @@ -79,6 +79,7 @@
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.EsField;
import org.elasticsearch.xpack.sql.util.CollectionUtils;

import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -408,6 +409,25 @@ public void testSimplifyCoalesceNulls() {
assertEquals(0, e.children().size());
}

public void testSimplifyCoalesceRandomNulls() {
Expression e = new SimplifyCoalesce().rule(new Coalesce(EMPTY, randomListOfNulls()));
assertEquals(Coalesce.class, e.getClass());
assertEquals(0, e.children().size());
}

public void testSimplifyCoalesceRandomNullsWithValue() {
Expression e = new SimplifyCoalesce().rule(new Coalesce(EMPTY,
CollectionUtils.combine(
CollectionUtils.combine(randomListOfNulls(), Literal.TRUE, Literal.FALSE, Literal.TRUE),
randomListOfNulls())));
assertEquals(1, e.children().size());
assertEquals(Literal.TRUE, e.children().get(0));
}

private List<Expression> randomListOfNulls() {
return asList(randomArray(1, 10, i -> new Literal[i], () -> Literal.NULL));
}

public void testSimplifyCoalesceFirstLiteral() {
Expression e = new SimplifyCoalesce()
.rule(new Coalesce(EMPTY,
Expand Down

0 comments on commit 65605e8

Please sign in to comment.