Skip to content

Commit 5d306ae

Browse files
committed
SQL: Fix issue with CASE/IIF pre-calculating results (#49553)
Previously, CaseProcessor was pre-calculating (called `process()`) on all the building elements of a CASE/IIF expression, not only the conditions involved but also the results, as well as the final else result. In case one of those results had an erroneous calculation (e.g.: division by zero) this was executed and resulted in an Exception to be thrown, even if this result was not used because of the condition guarding it. e.g.: ``` SELECT CASE myField1 = 0 THEN NULL ELSE myField2 / myField1 END FROM test; ``` Fixes: #49388 (cherry picked from commit dbd169a)
1 parent a4208e4 commit 5d306ae

File tree

3 files changed

+37
-6
lines changed

3 files changed

+37
-6
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,19 @@ FROM test_emp ORDER BY 1 LIMIT 5;
6969
10005 | 1 | 10005
7070
;
7171

72+
caseWithErroneousResultsForFalseConditions
73+
schema::bytes_in:i|bytes_out:i|div:i
74+
SELECT bytes_in, bytes_out, CASE WHEN bytes_in = 0 THEN NULL WHEN bytes_in < 10 THEN bytes_in * 20 ELSE bytes_out / bytes_in END div FROM logs ORDER BY bytes_in LIMIT 5;
75+
76+
bytes_in | bytes_out | div
77+
---------------+---------------+---------------
78+
0 |128 |null
79+
0 |null |null
80+
8 |null |160
81+
8 |null |160
82+
8 |null |160
83+
;
84+
7285
caseWhere
7386
SELECT last_name FROM test_emp WHERE CASE WHEN LENGTH(last_name) < 7 THEN 'ShortName' ELSE 'LongName' END = 'LongName'
7487
ORDER BY emp_no LIMIT 10;
@@ -265,6 +278,19 @@ SELECT emp_no, IIF(NULL, emp_no) AS IIF_1, IIF(NULL, emp_no, languages) AS IIF_2
265278
10005 | null | 1 | 10005
266279
;
267280

281+
iifWithErroneousResultsForFalseCondition
282+
schema::bytes_in:i|bytes_out:i|div:i
283+
SELECT bytes_in, bytes_out, IIF(bytes_in < 10, IIF(bytes_in = 0, NULL, bytes_in * 10), bytes_out / bytes_in) div FROM logs ORDER BY bytes_in LIMIT 5;
284+
285+
bytes_in | bytes_out | div
286+
---------------+---------------+---------------
287+
0 |128 |null
288+
0 |null |null
289+
8 |null |80
290+
8 |null |80
291+
8 |null |80
292+
;
293+
268294
iifWhere
269295
SELECT last_name FROM test_emp WHERE IIF(LENGTH(last_name) < 7, 'ShortName') IS NOT NULL ORDER BY emp_no LIMIT 10;
270296

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ id,@timestamp,bytes_in,bytes_out,client_ip,client_port,dest_ip,status
2525
24,2017-11-10T20:34:43Z,8,,10.0.1.166,,2001:cafe::13e1:16fc:8726:1bf8,OK
2626
25,2017-11-10T23:30:46Z,40,,10.0.1.199,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
2727
26,2017-11-10T21:13:16Z,20,,,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
28-
27,2017-11-10T23:36:32Z,0,,10.0.1.199,,2001:cafe::13e1:16fc:8726:1bf8,OK
28+
27,2017-11-10T23:36:32Z,0,128,10.0.1.199,,2001:cafe::13e1:16fc:8726:1bf8,OK
2929
28,2017-11-10T23:36:33Z,40,,10.0.1.199,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
3030
29,2017-11-10T20:35:26Z,20,,10.0.1.166,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
3131
30,2017-11-10T23:36:41Z,8,,,,2001:cafe::13e1:16fc:8726:1bf8,OK

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
1111

1212
import java.io.IOException;
13-
import java.util.ArrayList;
1413
import java.util.List;
1514
import java.util.Objects;
1615

@@ -40,14 +39,20 @@ public void writeTo(StreamOutput out) throws IOException {
4039

4140
@Override
4241
public Object process(Object input) {
43-
List<Object> objects = new ArrayList<>(processors.size());
44-
for (Processor processor : processors) {
45-
objects.add(processor.process(input));
42+
// Check every condition in sequence and if it evaluates to TRUE,
43+
// evaluate and return the result associated with that condition.
44+
for (int i = 0; i < processors.size() - 2; i += 2) {
45+
if (processors.get(i).process(input) == Boolean.TRUE) {
46+
return processors.get(i + 1).process(input);
47+
}
4648
}
47-
return apply(objects);
49+
// resort to default value
50+
return processors.get(processors.size() - 1).process(input);
4851
}
4952

5053
public static Object apply(List<Object> objects) {
54+
// Check every condition in sequence and if it evaluates to TRUE,
55+
// evaluate and return the result associated with that condition.
5156
for (int i = 0; i < objects.size() - 2; i += 2) {
5257
if (objects.get(i) == Boolean.TRUE) {
5358
return objects.get(i + 1);

0 commit comments

Comments
 (0)