Browse Source

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
Marios Trivyzas 5 years ago
parent
commit
dbd169afc9

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

@@ -69,6 +69,19 @@ FROM test_emp ORDER BY 1 LIMIT 5;
 10005   | 1      | 10005
 ;
 
+caseWithErroneousResultsForFalseConditions
+schema::bytes_in:i|bytes_out:i|div:i
+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;
+
+   bytes_in    |   bytes_out   |      div
+---------------+---------------+---------------
+0              |128            |null
+0              |null           |null
+8              |null           |160
+8              |null           |160
+8              |null           |160
+;
+
 caseWhere
 SELECT last_name FROM test_emp WHERE CASE WHEN LENGTH(last_name) < 7 THEN 'ShortName' ELSE 'LongName' END = 'LongName'
 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
 10005   | null  | 1     | 10005
 ;
 
+iifWithErroneousResultsForFalseCondition
+schema::bytes_in:i|bytes_out:i|div:i
+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;
+
+   bytes_in    |   bytes_out   |      div
+---------------+---------------+---------------
+0              |128            |null
+0              |null           |null
+8              |null           |80
+8              |null           |80
+8              |null           |80
+;
+
 iifWhere
 SELECT last_name FROM test_emp WHERE IIF(LENGTH(last_name) < 7, 'ShortName') IS NOT NULL ORDER BY emp_no LIMIT 10;
 

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

@@ -25,7 +25,7 @@ id,@timestamp,bytes_in,bytes_out,client_ip,client_port,dest_ip,status
 24,2017-11-10T20:34:43Z,8,,10.0.1.166,,2001:cafe::13e1:16fc:8726:1bf8,OK
 25,2017-11-10T23:30:46Z,40,,10.0.1.199,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
 26,2017-11-10T21:13:16Z,20,,,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
-27,2017-11-10T23:36:32Z,0,,10.0.1.199,,2001:cafe::13e1:16fc:8726:1bf8,OK
+27,2017-11-10T23:36:32Z,0,128,10.0.1.199,,2001:cafe::13e1:16fc:8726:1bf8,OK
 28,2017-11-10T23:36:33Z,40,,10.0.1.199,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
 29,2017-11-10T20:35:26Z,20,,10.0.1.166,,2001:cafe::ff07:bdcc:bc59:ff9f,OK
 30,2017-11-10T23:36:41Z,8,,,,2001:cafe::13e1:16fc:8726:1bf8,OK

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

@@ -10,7 +10,6 @@ import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
@@ -40,14 +39,20 @@ public class CaseProcessor implements Processor {
 
     @Override
     public Object process(Object input) {
-        List<Object> objects = new ArrayList<>(processors.size());
-        for (Processor processor : processors) {
-            objects.add(processor.process(input));
+        // Check every condition in sequence and if it evaluates to TRUE,
+        // evaluate and return the result associated with that condition.
+        for (int i = 0; i < processors.size() - 2; i += 2) {
+            if (processors.get(i).process(input) == Boolean.TRUE) {
+                return processors.get(i + 1).process(input);
+            }
         }
-        return apply(objects);
+        // resort to default value
+        return processors.get(processors.size() - 1).process(input);
     }
 
     public static Object apply(List<Object> objects) {
+        // Check every condition in sequence and if it evaluates to TRUE,
+        // evaluate and return the result associated with that condition.
         for (int i = 0; i < objects.size() - 2; i += 2) {
             if (objects.get(i) == Boolean.TRUE) {
                 return objects.get(i + 1);