Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue with CASE/IIF pre-calculating results #49553

Merged
merged 3 commits into from
Nov 26, 2019
Merged

Fix issue with CASE/IIF pre-calculating results #49553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca8f8b4
SQL: Fix issue with CASE/IIF pre-calculating results
matriv Nov 25, 2019
6aafde9
Address comments
matriv Nov 25, 2019
ca078eb
fix integ test
matriv Nov 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/conditionals.csv-spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/sql/qa/src/main/resources/logs.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 10 additions & 5 deletions ...main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/CaseProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
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;

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

@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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, here you basically take a condition and you evaluate it, and if it holds then you evaluate its results and break out of the loop. If it doesn't hold (evaluates to false), then move on to the next set of [condition, result] and repeat the process. Can you add a comment about this, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

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);
Expand Down