Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: Concat should be always not nullable #36601

Merged
merged 8 commits into from Dec 17, 2018
27 changes: 27 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/functions.csv-spec
Expand Up @@ -68,6 +68,33 @@ cct:s
AlejandroMcAlpine
;

selectConcatWithNullValues
SELECT first_name, CONCAT(first_name,null),last_name, CONCAT(null,null), LENGTH(CONCAT(null,null)) FROM test_emp ORDER BY first_name DESC LIMIT 20;

first_name:s |CONCAT(first_name,null):s| last_name:s |CONCAT(null,null):s|LENGTH(CONCAT(null,null)):i
---------------+-------------------------+----------------+-------------------+-------------------------
null | |Demeyer | |0
null | |Joslin | |0
null | |Reistad | |0
null | |Merlo | |0
null | |Swan | |0
null | |Chappelet | |0
null | |Portugali | |0
null | |Makrucki | |0
null | |Lortz | |0
null | |Brender | |0
Zvonko |Zvonko |Nyanchama | |0
Zhongwei |Zhongwei |Rosen | |0
Yongqiao |Yongqiao |Berztiss | |0
Yishay |Yishay |Tzvieli | |0
Yinghua |Yinghua |Dredge | |0
Xinglin |Xinglin |Eugenio | |0
Weiyi |Weiyi |Meriste | |0
Vishv |Vishv |Zockler | |0
Valter |Valter |Sullins | |0
Valdiodio |Valdiodio |Niizuma | |0
;

selectAsciiOfConcatWithGroupByOrderByCount
SELECT ASCII(CONCAT("first_name","last_name")) ascii, COUNT(*) count FROM "test_emp" GROUP BY ASCII(CONCAT("first_name","last_name")) ORDER BY ASCII(CONCAT("first_name","last_name")) DESC LIMIT 10;

Expand Down
Expand Up @@ -51,7 +51,7 @@ protected Pipe makePipe() {

@Override
public boolean nullable() {
return left().nullable() && right().nullable();
return false;
}

@Override
Expand Down
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.xpack.sql.expression.function.scalar.math.E;
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Floor;
import org.elasticsearch.xpack.sql.expression.function.scalar.string.Ascii;
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;
Expand Down Expand Up @@ -111,6 +112,8 @@ public class OptimizerTests extends ESTestCase {
private static final Literal FOUR = L(4);
private static final Literal FIVE = L(5);
private static final Literal SIX = L(6);

private static final String EMPTY_STRING = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it already available in TestUtils?


public static class DummyBooleanExpression extends Expression {

Expand Down Expand Up @@ -520,6 +523,13 @@ public void testSimplifyLeastRandomNullsWithValue() {
assertEquals(ONE, e.children().get(0));
assertEquals(TWO, e.children().get(1));
}

public void testConcatFoldingIsNotNull() {
FoldNull foldNull = new FoldNull();
assertEquals(1, foldNull.rule(new Concat(EMPTY, Literal.NULL, ONE)).fold());
assertEquals(1, foldNull.rule(new Concat(EMPTY, ONE, Literal.NULL)).fold());
assertEquals(EMPTY_STRING, foldNull.rule(new Concat(EMPTY, Literal.NULL, Literal.NULL)).fold());
}

//
// Logical simplifications
Expand Down
Expand Up @@ -263,4 +263,14 @@ public void testGroupKeyTypes_Date() {
assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->"));
assertThat(ee.output().get(1).toString(), startsWith("a{s->"));
}

public void testConcatIsNotFoldedForNull() {
PhysicalPlan p = plan("SELECT keyword FROM test WHERE CONCAT(keyword, null) IS NULL");
assertEquals(LocalExec.class, p.getClass());
LocalExec le = (LocalExec) p;
assertEquals(EmptyExecutable.class, le.executable().getClass());
EmptyExecutable ee = (EmptyExecutable) le.executable();
assertEquals(1, ee.output().size());
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
}
}