Skip to content

Commit 2ed6ab9

Browse files
authored
SQL: Concat should be always not nullable (#36601)
1 parent b376edf commit 2ed6ab9

File tree

4 files changed

+47
-1
lines changed

4 files changed

+47
-1
lines changed

x-pack/plugin/sql/qa/src/main/resources/functions.csv-spec

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,33 @@ cct:s
6868
AlejandroMcAlpine
6969
;
7070

71+
selectConcatWithNullValues
72+
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;
73+
74+
first_name:s |CONCAT(first_name,null):s| last_name:s |CONCAT(null,null):s|LENGTH(CONCAT(null,null)):i
75+
---------------+-------------------------+----------------+-------------------+-------------------------
76+
null | |Demeyer | |0
77+
null | |Joslin | |0
78+
null | |Reistad | |0
79+
null | |Merlo | |0
80+
null | |Swan | |0
81+
null | |Chappelet | |0
82+
null | |Portugali | |0
83+
null | |Makrucki | |0
84+
null | |Lortz | |0
85+
null | |Brender | |0
86+
Zvonko |Zvonko |Nyanchama | |0
87+
Zhongwei |Zhongwei |Rosen | |0
88+
Yongqiao |Yongqiao |Berztiss | |0
89+
Yishay |Yishay |Tzvieli | |0
90+
Yinghua |Yinghua |Dredge | |0
91+
Xinglin |Xinglin |Eugenio | |0
92+
Weiyi |Weiyi |Meriste | |0
93+
Vishv |Vishv |Zockler | |0
94+
Valter |Valter |Sullins | |0
95+
Valdiodio |Valdiodio |Niizuma | |0
96+
;
97+
7198
selectAsciiOfConcatWithGroupByOrderByCount
7299
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;
73100

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ protected Pipe makePipe() {
5151

5252
@Override
5353
public boolean nullable() {
54-
return left().nullable() && right().nullable();
54+
return false;
5555
}
5656

5757
@Override

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.xpack.sql.expression.function.scalar.math.E;
3535
import org.elasticsearch.xpack.sql.expression.function.scalar.math.Floor;
3636
import org.elasticsearch.xpack.sql.expression.function.scalar.string.Ascii;
37+
import org.elasticsearch.xpack.sql.expression.function.scalar.string.Concat;
3738
import org.elasticsearch.xpack.sql.expression.function.scalar.string.Repeat;
3839
import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator;
3940
import org.elasticsearch.xpack.sql.expression.predicate.Range;
@@ -87,6 +88,7 @@
8788
import org.elasticsearch.xpack.sql.type.DataType;
8889
import org.elasticsearch.xpack.sql.type.EsField;
8990
import org.elasticsearch.xpack.sql.util.CollectionUtils;
91+
import org.elasticsearch.xpack.sql.util.StringUtils;
9092

9193
import java.util.Arrays;
9294
import java.util.Collections;
@@ -519,6 +521,13 @@ public void testSimplifyLeastRandomNullsWithValue() {
519521
assertEquals(ONE, e.children().get(0));
520522
assertEquals(TWO, e.children().get(1));
521523
}
524+
525+
public void testConcatFoldingIsNotNull() {
526+
FoldNull foldNull = new FoldNull();
527+
assertEquals(1, foldNull.rule(new Concat(EMPTY, Literal.NULL, ONE)).fold());
528+
assertEquals(1, foldNull.rule(new Concat(EMPTY, ONE, Literal.NULL)).fold());
529+
assertEquals(StringUtils.EMPTY, foldNull.rule(new Concat(EMPTY, Literal.NULL, Literal.NULL)).fold());
530+
}
522531

523532
//
524533
// Logical simplifications

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,4 +263,14 @@ public void testGroupKeyTypes_Date() {
263263
assertThat(ee.output().get(0).toString(), startsWith("COUNT(1){a->"));
264264
assertThat(ee.output().get(1).toString(), startsWith("a{s->"));
265265
}
266+
267+
public void testConcatIsNotFoldedForNull() {
268+
PhysicalPlan p = plan("SELECT keyword FROM test WHERE CONCAT(keyword, null) IS NULL");
269+
assertEquals(LocalExec.class, p.getClass());
270+
LocalExec le = (LocalExec) p;
271+
assertEquals(EmptyExecutable.class, le.executable().getClass());
272+
EmptyExecutable ee = (EmptyExecutable) le.executable();
273+
assertEquals(1, ee.output().size());
274+
assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#"));
275+
}
266276
}

0 commit comments

Comments
 (0)