honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix bug with optimization of null related conditionals (#41355) 

The SimplifyConditional rule is removing NULL literals from those
functions to simplify their evaluation. This happens in the Optimizer
and a new instance of the conditional function is generated. Previously,
the dataType was not set properly (defaulted to DataType.NULL) for
those new instances and since the resolveType() wasn't called again
it resulted in returning always null.

E.g.:

SELECT COALESCE(null, 'foo', null, 'bar')

COALESCE(null, 'foo', null, 'bar')
-----------------
null

This issue was not visible before because the tests always used an alias
for the conditional function which caused the resolveType() to be
called which sets the dataType properly.

E.g.:

SELECT COALESCE(null, 'foo', null, 'bar') as c

c
-----------------
foo

(cherry picked from commit c39980a65dd593363f1d8d1b038b26cb0ce02aaf)
This commit is contained in:
Marios Trivyzas 2019-04-19 19:03:28 +03:00
parent d989079df5
commit 7a34ba35f7
No known key found for this signature in database
GPG Key ID: 8817B46B0CF36A3F
3 changed files with 20 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
x-pack/plugin/sql/qa/src/main/resources/null.csv-spec
View File

@ -61,6 +61,13 @@ c:i
;
coalesceMixed
SELECT COALESCE(null, 123, null, 321);
COALESCE(null, 123, null, 321):i
123
;
coalesceMixedWithAlias
SELECT COALESCE(null, 123, null, 321) AS c;
c:i

9
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java
View File

@ -25,7 +25,7 @@ import static org.elasticsearch.xpack.sql.util.StringUtils.ordinal;
*/
public abstract class ConditionalFunction extends ScalarFunction {
protected DataType dataType = DataType.NULL;
protected DataType dataType = null;
ConditionalFunction(Source source, List<Expression> fields) {
super(source, fields);
@ -33,6 +33,12 @@ public abstract class ConditionalFunction extends ScalarFunction {
@Override
public DataType dataType() {
if (dataType == null) {
dataType = DataType.NULL;
for (Expression exp : children()) {
dataType = DataTypeConversion.commonType(dataType, exp.dataType());
}
}
return dataType;
}
@ -61,7 +67,6 @@ public abstract class ConditionalFunction extends ScalarFunction {
child.dataType().typeName));
}
}
dataType = DataTypeConversion.commonType(dataType, child.dataType());
}
return TypeResolution.TYPE_RESOLVED;
}

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

@ -501,6 +501,7 @@ public class OptimizerTests extends ESTestCase {
randomListOfNulls())));
assertEquals(1, e.children().size());
assertEquals(TRUE, e.children().get(0));
assertEquals(DataType.BOOLEAN, e.dataType());
}
private List<Expression> randomListOfNulls() {
@ -514,6 +515,7 @@ public class OptimizerTests extends ESTestCase {
assertEquals(Coalesce.class, e.getClass());
assertEquals(1, e.children().size());
assertEquals(TRUE, e.children().get(0));
assertEquals(DataType.BOOLEAN, e.dataType());
}
public void testSimplifyIfNullNulls() {
@ -527,11 +529,13 @@ public class OptimizerTests extends ESTestCase {
assertEquals(IfNull.class, e.getClass());
assertEquals(1, e.children().size());
assertEquals(ONE, e.children().get(0));
assertEquals(DataType.INTEGER, e.dataType());
e = new SimplifyConditional().rule(new IfNull(EMPTY, ONE, NULL));
assertEquals(IfNull.class, e.getClass());
assertEquals(1, e.children().size());
assertEquals(ONE, e.children().get(0));
assertEquals(DataType.INTEGER, e.dataType());
}
public void testFoldNullNotAppliedOnNullIf() {
@ -559,6 +563,7 @@ public class OptimizerTests extends ESTestCase {
assertEquals(2, e.children().size());
assertEquals(ONE, e.children().get(0));
assertEquals(TWO, e.children().get(1));
assertEquals(DataType.INTEGER, e.dataType());
}
public void testSimplifyLeastNulls() {
@ -580,6 +585,7 @@ public class OptimizerTests extends ESTestCase {
assertEquals(2, e.children().size());
assertEquals(ONE, e.children().get(0));
assertEquals(TWO, e.children().get(1));
assertEquals(DataType.INTEGER, e.dataType());
}
public void testConcatFoldingIsNotNull() {