Skip to content

Commit fd0affb

Browse files
committed
SQL: Fix issue with IIF function when condition folds (#46290)
Previously, when the condition (1st argument) of the IIF function could be evaluated (folded) to false, the `IfConditional` was eliminated which caused `IndexOutOfBoundsException` to be thrown when `info()` and `resolveType()` methods where called. Fixes: #46268 (cherry picked from commit 9a885a3)
1 parent 3f67cbe commit fd0affb

File tree

5 files changed

+62
-3
lines changed

5 files changed

+62
-3
lines changed

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,21 @@ ORDER BY emp_no LIMIT 10;
5252
10014 | null
5353
;
5454

55+
caseWithConditionsFolded
56+
schema::emp_no:i|CASE_1:byte|CASE_2:i
57+
SELECT emp_no, CASE WHEN NULL = 1 THEN emp_no WHEN 10 < 5 THEN emp_no ELSE languages END AS "CASE_1",
58+
CASE WHEN NULL = 1 THEN languages WHEN 10 > 5 THEN emp_no ELSE languages END AS "CASE_2"
59+
FROM test_emp ORDER BY 1 LIMIT 5;
60+
61+
emp_no | CASE_1 | CASE_2
62+
--------+--------+-------
63+
10001 | 2 | 10001
64+
10002 | 5 | 10002
65+
10003 | 4 | 10003
66+
10004 | 5 | 10004
67+
10005 | 1 | 10005
68+
;
69+
5570
caseWhere
5671
SELECT last_name FROM test_emp WHERE CASE WHEN LENGTH(last_name) < 7 THEN 'ShortName' ELSE 'LongName' END = 'LongName'
5772
ORDER BY emp_no LIMIT 10;
@@ -235,6 +250,19 @@ ORDER BY emp_no LIMIT 10;
235250
10014 | null
236251
;
237252

253+
iifWithConditionFolded
254+
schema::emp_no:i|IIF_1:i|IIF_2:byte|IIF_3:i
255+
SELECT emp_no, IIF(NULL, emp_no) AS IIF_1, IIF(NULL, emp_no, languages) AS IIF_2, IIF(10 > 5, emp_no, languages) AS IIF_3 FROM test_emp ORDER BY 1 LIMIT 5;
256+
257+
emp_no | IIF_1 | IIF_2 | IIF_3
258+
--------+-------+-------+------
259+
10001 | null | 2 | 10001
260+
10002 | null | 5 | 10002
261+
10003 | null | 4 | 10003
262+
10004 | null | 5 | 10004
263+
10005 | null | 1 | 10005
264+
;
265+
238266
iifWhere
239267
SELECT last_name FROM test_emp WHERE IIF(LENGTH(last_name) < 7, 'ShortName') IS NOT NULL ORDER BY emp_no LIMIT 10;
240268

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Iif.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,21 @@
1818

1919
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
2020
import static org.elasticsearch.xpack.sql.expression.TypeResolutions.isBoolean;
21+
import static org.elasticsearch.xpack.sql.util.CollectionUtils.combine;
2122

2223
public class Iif extends Case {
2324

2425
public Iif(Source source, Expression condition, Expression thenResult, Expression elseResult) {
2526
super(source, Arrays.asList(new IfConditional(source, condition, thenResult), elseResult != null ? elseResult : Literal.NULL));
2627
}
2728

28-
private Iif(Source source, List<Expression> expressions) {
29+
Iif(Source source, List<Expression> expressions) {
2930
super(source, expressions);
3031
}
3132

3233
@Override
3334
protected NodeInfo<? extends Iif> info() {
34-
return NodeInfo.create(this, Iif::new, conditions().get(0).condition(), conditions().get(0).result(), elseResult());
35+
return NodeInfo.create(this, Iif::new, combine(conditions(), elseResult()));
3536
}
3637

3738
@Override
@@ -41,6 +42,10 @@ public Expression replaceChildren(List<Expression> newChildren) {
4142

4243
@Override
4344
protected TypeResolution resolveType() {
45+
if (conditions().isEmpty()) {
46+
return TypeResolution.TYPE_RESOLVED;
47+
}
48+
4449
TypeResolution conditionTypeResolution = isBoolean(conditions().get(0).condition(), sourceText(), Expressions.ParamOrdinal.FIRST);
4550
if (conditionTypeResolution.unresolved()) {
4651
return conditionTypeResolution;

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CaseTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717

1818
import java.util.ArrayList;
1919
import java.util.Arrays;
20+
import java.util.Collections;
2021
import java.util.List;
2122
import java.util.Objects;
2223

24+
import static org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
2325
import static org.elasticsearch.xpack.sql.expression.function.scalar.FunctionTestUtils.randomIntLiteral;
2426
import static org.elasticsearch.xpack.sql.expression.function.scalar.FunctionTestUtils.randomStringLiteral;
2527
import static org.elasticsearch.xpack.sql.tree.Source.EMPTY;
@@ -117,6 +119,13 @@ public void testDataTypes() {
117119
assertEquals(DataType.KEYWORD, c.dataType());
118120
}
119121

122+
public void testAllConditionsFolded() {
123+
Case c = new Case(EMPTY, Collections.singletonList(Literal.of(EMPTY, "foo")));
124+
assertEquals(DataType.KEYWORD, c.dataType());
125+
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
126+
assertNotNull(c.info());
127+
}
128+
120129
private List<Expression> mutateChildren(Case c) {
121130
boolean removeConditional = randomBoolean();
122131
List<Expression> expressions = new ArrayList<>(c.children().size());

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/IifTests.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,24 @@
66
package org.elasticsearch.xpack.sql.expression.predicate.conditional;
77

88
import org.elasticsearch.xpack.sql.expression.Expression;
9+
import org.elasticsearch.xpack.sql.expression.Literal;
910
import org.elasticsearch.xpack.sql.expression.function.scalar.FunctionTestUtils;
1011
import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals;
1112
import org.elasticsearch.xpack.sql.tree.AbstractNodeTestCase;
1213
import org.elasticsearch.xpack.sql.tree.NodeSubclassTests;
1314
import org.elasticsearch.xpack.sql.tree.Source;
1415
import org.elasticsearch.xpack.sql.tree.SourceTests;
16+
import org.elasticsearch.xpack.sql.type.DataType;
1517

1618
import java.util.ArrayList;
1719
import java.util.Arrays;
20+
import java.util.Collections;
1821
import java.util.List;
1922
import java.util.Objects;
2023

2124
import static org.elasticsearch.xpack.sql.expression.function.scalar.FunctionTestUtils.randomIntLiteral;
2225
import static org.elasticsearch.xpack.sql.expression.function.scalar.FunctionTestUtils.randomStringLiteral;
26+
import static org.elasticsearch.xpack.sql.tree.Source.EMPTY;
2327
import static org.elasticsearch.xpack.sql.tree.SourceTests.randomSource;
2428

2529
/**
@@ -74,6 +78,13 @@ public void testReplaceChildren() {
7478
newChildren.get(2))));
7579
}
7680

81+
public void testConditionFolded() {
82+
Iif iif = new Iif(EMPTY, Collections.singletonList(Literal.of(EMPTY, "foo")));
83+
assertEquals(DataType.KEYWORD, iif.dataType());
84+
assertEquals(Expression.TypeResolution.TYPE_RESOLVED, iif.typeResolved());
85+
assertNotNull(iif.info());
86+
}
87+
7788
private List<Expression> mutateChildren(Iif iif) {
7889
List<Expression> expressions = new ArrayList<>(3);
7990
Equals eq = (Equals) iif.conditions().get(0).condition();

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@
4949
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce;
5050
import org.elasticsearch.xpack.sql.expression.predicate.conditional.ConditionalFunction;
5151
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest;
52-
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Iif;
5352
import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfConditional;
5453
import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull;
54+
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Iif;
5555
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Least;
5656
import org.elasticsearch.xpack.sql.expression.predicate.conditional.NullIf;
5757
import org.elasticsearch.xpack.sql.expression.predicate.logical.And;
@@ -112,6 +112,7 @@
112112
import static java.util.Collections.emptyList;
113113
import static java.util.Collections.emptyMap;
114114
import static java.util.Collections.singletonList;
115+
import static org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
115116
import static org.elasticsearch.xpack.sql.expression.Literal.FALSE;
116117
import static org.elasticsearch.xpack.sql.expression.Literal.NULL;
117118
import static org.elasticsearch.xpack.sql.expression.Literal.TRUE;
@@ -630,6 +631,7 @@ public void testSimplifyCaseConditionsFoldWhenFalse() {
630631
assertThat(c.conditions().get(0).condition().toString(), startsWith("Equals[a{f}#"));
631632
assertThat(c.conditions().get(1).condition().toString(), startsWith("GreaterThan[a{f}#"));
632633
assertFalse(c.foldable());
634+
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
633635
}
634636

635637
public void testSimplifyCaseConditionsFoldWhenTrue() {
@@ -661,6 +663,7 @@ public void testSimplifyCaseConditionsFoldWhenTrue() {
661663
assertThat(c.conditions().get(0).condition().toString(), startsWith("Equals[a{f}#"));
662664
assertThat(c.conditions().get(1).condition().toString(), startsWith("Equals[=1,=1]#"));
663665
assertFalse(c.foldable());
666+
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
664667
}
665668

666669
public void testSimplifyCaseConditionsFoldCompletely() {
@@ -685,6 +688,7 @@ public void testSimplifyCaseConditionsFoldCompletely() {
685688
assertThat(c.conditions().get(0).condition().toString(), startsWith("Equals[=1,=1]#"));
686689
assertTrue(c.foldable());
687690
assertEquals("foo2", c.fold());
691+
assertEquals(TypeResolution.TYPE_RESOLVED, c.typeResolved());
688692
}
689693

690694
public void testSimplifyIif_ConditionTrue() {
@@ -696,6 +700,7 @@ public void testSimplifyIif_ConditionTrue() {
696700
assertEquals(1, iif.conditions().size());
697701
assertTrue(iif.foldable());
698702
assertEquals("foo", iif.fold());
703+
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
699704
}
700705

701706
public void testSimplifyIif_ConditionFalse() {
@@ -707,6 +712,7 @@ public void testSimplifyIif_ConditionFalse() {
707712
assertEquals(0, iif.conditions().size());
708713
assertTrue(iif.foldable());
709714
assertEquals("bar", iif.fold());
715+
assertEquals(TypeResolution.TYPE_RESOLVED, iif.typeResolved());
710716
}
711717

712718
//

0 commit comments

Comments
 (0)