SQL: Implement NULLIF(expr1, expr2) function#35826
Conversation
NULLIF returns null if the 2 expressions are equal or the expr1 otherwise. Closes: elastic#35818
|
Pinging @elastic/es-search |
astefan
left a comment
There was a problem hiding this comment.
Looking good. Left few comments.
| <2> 2nd expression | ||
|
|
||
|
|
||
| *Output*: `NULL` if the 2 expressions are equal, otherwise the 1st expression. |
There was a problem hiding this comment.
Lowercase null? For another function doc page you used the lowercased version of null: https://github.com/elastic/elasticsearch/pull/35793/files#diff-cf102e856ee9e39135c9677a52db35e1R108
| /** | ||
| * Name is `NULLIf` to avoid having it registered as `NULL_IF` instead of `NULLIF`. | ||
| */ | ||
| public class NULLIf extends ConditionalFunction { |
There was a problem hiding this comment.
Can we do something about these class names? I understand why it is named like this and same goes for IFNull added previously, but I believe we should change the way we normalize the names. Simplest/quickest approach: put these exception functions in a Set (or similar) and treat them differently in the normalize method from FunctionRegistry.
| } | ||
|
|
||
| public static Object apply(Object leftValue, Object rightValue) { | ||
| if (BinaryComparisonProcessor.BinaryComparisonOperation.EQ.apply(leftValue, rightValue) == Boolean.TRUE) { |
There was a problem hiding this comment.
Maybe import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor.BinaryComparisonOperation to shorten this line and make it more readable?
|
|
||
| } else if (e instanceof NULLIf) { | ||
| return e; // Special rule SimplifyNullIf is applied instead | ||
|
|
There was a problem hiding this comment.
Was on purpose as we have them in all else if blocks.
|
Thanks @astefan! Pushed commit to address your comments. |
|
retest this please |
|
|
||
| } else if (e instanceof NULLIf) { | ||
| return e; // Special rule SimplifyNullIf is applied instead | ||
|
|
NULLIF returns null if the 2 expressions are equal or the expr1 otherwise. Closes: #35818
|
Backported to |
NULLIF returns null if the 2 expressions are equal or the
expr1 otherwise.
Closes: #35818