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
ESQL: allow null
in date math
#103610
ESQL: allow null
in date math
#103610
Conversation
This fixes `null`'s handling in date math. So far the `null` (of type NULL) has been rejected by the type resolution. This is now allowed through, leading to a `null` result, inline with the other types.
Documentation preview: |
Hi @bpintea, I've created a changelog YAML for you. |
Pinging @elastic/es-ql (Team:QL) |
Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL) |
Pinging @elastic/es-analytics-geo (Team:Analytics) |
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
@@ -177,6 +177,10 @@ public static boolean isDateTime(DataType type) { | |||
return type == DATETIME; | |||
} | |||
|
|||
public static boolean isNullOrDateTime(DataType type) { |
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.
Since there's already EsqlDataTypes I would remove this method.
Zooming out a bit having both isType and isNullOrType is confusing ? if the null check is so common, why not do it in a generic way instead of having different method variants.
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.
👍
There's also isNullOrNumeric()
, so i kept the style, but in fact this method's no longer needed, so I removed it.
💔 Backport failed
You can use sqren/backport to manually backport by running |
This fixes `null`'s handling in date math. So far the `null` (of type `NULL`) has been rejected by the type resolution. This is now allowed through, leading to a `null` result, inline with the other types. Fixes elastic#103085.
This fixes
null
's handling in date math. So far thenull
(of typeNULL
) has been rejected by the type resolution. This is now allowed through, leading to anull
result, inline with the other types.Fixes #103085.