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

SQL: division by zero even if conditional function should have protected from this #49388

Closed
astefan opened this issue Nov 20, 2019 · 5 comments · Fixed by #49553
Closed

SQL: division by zero even if conditional function should have protected from this #49388

astefan opened this issue Nov 20, 2019 · 5 comments · Fixed by #49553
Assignees
@matriv
Labels
:Analytics/SQL SQL querying >bug

Comments

@astefan
Copy link
Contributor

astefan commented Nov 20, 2019

SELECT CASE WHEN languages = 0 THEN NULL ELSE (salary / languages) END FROM test_emp

results in

java.lang.ArithmeticException: / by zero
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Arithmetics.div(Arithmetics.java:113) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor$NumericArithmetic.wrap(BinaryArithmeticProcessor.java:29) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor$BinaryArithmeticOperation.doApply(BinaryArithmeticProcessor.java:131) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.PredicateBiFunction.apply(PredicateBiFunction.java:24) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.doProcess(BinaryArithmeticProcessor.java:176) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.gen.processor.BinaryProcessor.process(BinaryProcessor.java:50) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.conditional.CaseProcessor.process(CaseProcessor.java:45) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.extractor.ComputingExtractor.extract(ComputingExtractor.java:78) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.SearchHitRowSet.extractValue(SearchHitRowSet.java:128) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.SearchHitRowSet.extractValue(SearchHitRowSet.java:31) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.ResultRowSet.getColumn(ResultRowSet.java:37) ~[?:?]
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/SQL)

@matriv matriv self-assigned this Nov 21, 2019
@matriv
Copy link
Contributor

matriv commented Nov 21, 2019

@astefan I cannot reproduce this on master.

matriv added a commit to matriv/elasticsearch that referenced this issue Nov 25, 2019
@matriv
SQL: Fix issue with CASE/IIF pre-calculating results
ca8f8b4 
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 (if true/if false), 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: elastic#49388
@matriv matriv mentioned this issue Nov 25, 2019
Merged
@matriv
Copy link
Contributor

matriv commented Nov 25, 2019

The issue was only showing up if there was a division by integer 0 since in other cases there is no Exception thrown by Infinity is returned instead.

@matriv
Copy link
Contributor

matriv commented Nov 25, 2019

Not reproducable with test_emp as there is no entry with languages = 0 (only NULLS or >= 1.)

SELECT CASE WHEN languages = 0 THEN NULL ELSE (salary / languages) END FROM test_emp

results in

java.lang.ArithmeticException: / by zero
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.Arithmetics.div(Arithmetics.java:113) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor$NumericArithmetic.wrap(BinaryArithmeticProcessor.java:29) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor$BinaryArithmeticOperation.doApply(BinaryArithmeticProcessor.java:131) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.PredicateBiFunction.apply(PredicateBiFunction.java:24) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.doProcess(BinaryArithmeticProcessor.java:176) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.gen.processor.BinaryProcessor.process(BinaryProcessor.java:50) ~[?:?]
        at org.elasticsearch.xpack.sql.expression.predicate.conditional.CaseProcessor.process(CaseProcessor.java:45) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.extractor.ComputingExtractor.extract(ComputingExtractor.java:78) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.SearchHitRowSet.extractValue(SearchHitRowSet.java:128) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.SearchHitRowSet.extractValue(SearchHitRowSet.java:31) ~[?:?]
        at org.elasticsearch.xpack.sql.execution.search.ResultRowSet.getColumn(ResultRowSet.java:37) ~[?:?]

matriv added a commit that referenced this issue Nov 26, 2019
@matriv
SQL: Fix issue with CASE/IIF pre-calculating results (#49553)
dbd169a 
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
matriv added a commit that referenced this issue Nov 26, 2019
@matriv
SQL: Fix issue with CASE/IIF pre-calculating results (#49553)
5d306ae 
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)
matriv added a commit that referenced this issue Nov 26, 2019
@matriv
SQL: Fix issue with CASE/IIF pre-calculating results (#49553)
05eb6f3 
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)
matriv added a commit that referenced this issue Nov 26, 2019
@matriv
SQL: Fix issue with CASE/IIF pre-calculating results (#49553)
b500ccf 
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)
@matriv
Copy link
Contributor

matriv commented Nov 26, 2019

master : dbd169a
7.x : 5d306ae
7.5 : 05eb6f3
7.4 : b500ccf

@astefan astefan mentioned this issue Nov 28, 2019
Closed
This was referenced Feb 3, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matriv matriv

Labels
:Analytics/SQL SQL querying >bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix issue with CASE/IIF pre-calculating results matriv/elasticsearch
3 participants
@astefan @matriv @elasticmachine