-
Notifications
You must be signed in to change notification settings - Fork 25k
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.
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
Fix issue with CASE/IIF pre-calculating results #49553
Conversation
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
Pinging @elastic/es-search (:Search/SQL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left two cosmetic comments and in terms of tests, I would like to see an IT one with more than one WHEN
branch.
List<Object> objects = new ArrayList<>(processors.size()); | ||
for (Processor processor : processors) { | ||
objects.add(processor.process(input)); | ||
for (int i = 0; i < processors.size() - 2; i += 2) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
for (Processor processor : processors) { | ||
objects.add(processor.process(input)); | ||
for (int i = 0; i < processors.size() - 2; i += 2) { | ||
if (processors.get(i).process(input)==Boolean.TRUE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
processors.get(i).process(input) == Boolean.TRUE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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)
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)
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)
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.:
Fixes: #49388