Handle NULL arithmetic operations with INTERVALs#49633
Handle NULL arithmetic operations with INTERVALs#49633astefan merged 5 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-search (:Search/SQL) |
arithmetic operators are supported.
matriv
left a comment
There was a problem hiding this comment.
LGTM. Minor suggestion: Combine all null +/-/* interval in one query in integ tests.
Maybe two queries one for null on the left side one on the right, or maybe even 3 queries
each testing one operator.
|
@astefan thx! Adding the numeric-null operations will catch any future bugs in that area. |
costin
left a comment
There was a problem hiding this comment.
LGTM - do the other operators handle Null accordingly?
I wonder if going forward we could generalize this a bit since all operators would have to take into account null so maybe instead of checking explictly for null, we can do the check in the base TypeResolver base class and act accordingly so children would only have to worry about actual values.
|
@costin the other operators seem to handle NULL properly. There is a test I added for all the math operators with NULL combinations here https://github.com/elastic/elasticsearch/pull/49633/files#diff-c579a8eae7dda4bd86f6ce71c5bc0c36R24. |
(cherry picked from commit ce72761)
(cherry picked from commit ce72761)
Up until this change, NULL (as a data type) was not permitted in *, +, - operations involving intervals, an error message being generated.
Fixes #49297.